Skip to content

fix(tool): allow null for optional nested object fields#1170

Open
JGoP-L wants to merge 2 commits intoagentscope-ai:mainfrom
JGoP-L:fix/tool-null-optional-fields
Open

fix(tool): allow null for optional nested object fields#1170
JGoP-L wants to merge 2 commits intoagentscope-ai:mainfrom
JGoP-L:fix/tool-null-optional-fields

Conversation

@JGoP-L
Copy link
Copy Markdown
Contributor

@JGoP-L JGoP-L commented Apr 8, 2026

AgentScope-Java Version

1.0.12-SNAPSHOT

Description

Fixes #1163

Background

When a tool parameter contains a nested object, AgentScope-Java currently treats these two cases differently for optional fields:

  1. The optional field is omitted
  2. The optional field is explicitly set to null

Case 1 passes validation, but case 2 fails during JSON Schema validation with an error like:

null found, string expected

This makes tool calling brittle in practice, because LLMs often emit null for optional fields instead of omitting them entirely.

Purpose

Make optional nested object fields behave consistently:

  • missing optional field: valid
  • optional field explicitly set to null: also valid

Required field validation should continue to work as before.

Changes made

1. Fix tool input validation for explicit null optional fields

Updated ToolValidator.validateInput(...) to normalize tool input before JSON Schema validation.

It now recursively removes null-valued object properties before validating the payload. This means:

  • optional fields with null are treated the same as omitted fields
  • required fields with null still fail validation, because the field is removed and then checked by required

Modified file:

  • agentscope-core/src/main/java/io/agentscope/core/tool/ToolValidator.java

2. Add regression tests for generated nested bean schema

Added tests to verify:

  • missing optional nested bean field is accepted
  • explicit null optional nested bean field is accepted

Modified file:

  • agentscope-core/src/test/java/io/agentscope/core/tool/ToolValidatorTest.java

3. Add protection tests for object parameter conversion

Added tests to confirm that nested bean parameter conversion itself already works correctly for:

  • missing optional field
  • explicit null optional field

This verifies the bug is in validation behavior rather than conversion behavior.

Modified file:

  • agentscope-core/src/test/java/io/agentscope/core/tool/ToolMethodInvokerTest.java

How to test

Run:

mvn -pl agentscope-core -Dtest=ToolValidatorTest,ToolMethodInvokerTest test

@JGoP-L JGoP-L requested review from a team and Copilot April 8, 2026 15:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes tool input validation so that optional object fields explicitly set to null are treated the same as omitted fields, preventing brittle failures during JSON Schema validation (Fixes #1163).

Changes:

  • Normalize tool-call JSON inputs by pruning null object properties before running JSON Schema validation.
  • Add regression tests covering generated nested bean schemas accepting missing vs explicit-null optional fields.
  • Add protection tests confirming nested bean parameter conversion already handles missing vs explicit-null optional fields.

Reviewed changes

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

File Description
agentscope-core/src/main/java/io/agentscope/core/tool/ToolValidator.java Adds pre-validation normalization that prunes null-valued object properties before schema validation.
agentscope-core/src/test/java/io/agentscope/core/tool/ToolValidatorTest.java Adds regression tests for generated bean schemas around missing vs explicit null optional nested fields.
agentscope-core/src/test/java/io/agentscope/core/tool/ToolMethodInvokerTest.java Adds conversion tests showing bean parameter conversion behaves correctly for missing vs explicit null optional fields.

Comment on lines +105 to +109
/**
* Removes null-valued object properties before schema validation.
*
* <p>This treats explicit nulls the same as omitted optional fields, while still allowing
* required-field validation to fail naturally after the null-valued property is removed.
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalizeOptionalNullFields/pruneNullObjectFields removes all null-valued object properties before schema validation. This can inadvertently let invalid inputs pass when the schema is strict (e.g., additionalProperties: false): an unknown field provided as null will be removed prior to validation and therefore won't be reported as an additional/unknown property. Consider restricting pruning to schema-declared optional properties (schema-aware walk), or doing a two-pass validation where you only remove fields that failed specifically due to a null type mismatch and then re-validate (so additional-properties violations are still caught).

Copilot uses AI. Check for mistakes.
Comment on lines +126 to +129
if (node.isObject()) {
ObjectNode objectNode = (ObjectNode) node;
List<String> nullFieldNames = new java.util.ArrayList<>();
objectNode
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using fully-qualified collection types in new code (new java.util.ArrayList<>()). Please add an import java.util.ArrayList; and use new ArrayList<>() for consistency with the rest of the file and the project's Java standards.

Copilot uses AI. Check for mistakes.

String result = ToolValidator.validateInput(input, toolSchema);
assertNull(result, "Explicit null optional nested field should be accepted");
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new regression tests cover optional nested bean fields, but they don't assert the other half of the contract described in the PR: that a required nested bean field explicitly set to null still fails validation. Adding a test for {"payload":{"requiredField":null}} (or similar) would prevent future changes from accidentally making required-null pass due to pruning.

Suggested change
}
}
@Test
@DisplayName("Should reject explicit null required nested bean field")
void testGeneratedBeanSchema_ExplicitNullRequiredField() throws Exception {
Map<String, Object> toolSchema = buildBeanToolSchema();
String input = "{\"payload\":{\"requiredField\":null}}";
String result = ToolValidator.validateInput(input, toolSchema);
assertNotNull(result, "Explicit null required nested field should be rejected");
}

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 80.32787% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...in/java/io/agentscope/core/tool/ToolValidator.java 80.32% 4 Missing and 8 partials ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]:Tool parameter conversion bug

2 participants