diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java index fa7053246f..1c95d4e731 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java @@ -151,6 +151,13 @@ private Object[] buildMethodArguments(Map toolInputArguments, @N return toolContext; } Object rawArgument = toolInputArguments.get(parameter.getName()); + // Missing/null values cannot be unboxed to primitives by Method.invoke and + // would otherwise escape as a raw IllegalArgumentException (GH-6723). + if (rawArgument == null && parameter.getType().isPrimitive()) { + throw new ToolExecutionException(this.toolDefinition, + new IllegalArgumentException("Cannot pass a null value for primitive tool parameter '" + + parameter.getName() + "' of type " + parameter.getType().getName())); + } return buildTypedArgument(rawArgument, parameter.getParameterizedType()); }).toArray(); } @@ -188,6 +195,12 @@ private Object[] buildMethodArguments(Map toolInputArguments, @N catch (IllegalAccessException ex) { throw new IllegalStateException("Could not access method: " + ex.getMessage(), ex); } + catch (IllegalArgumentException ex) { + // Method.invoke throws IllegalArgumentException (not + // InvocationTargetException) + // for argument mismatches such as null-to-primitive; wrap for GH-6723. + throw new ToolExecutionException(this.toolDefinition, ex); + } catch (InvocationTargetException ex) { throw new ToolExecutionException(this.toolDefinition, ex.getCause()); } diff --git a/spring-ai-model/src/test/java/org/springframework/ai/tool/method/MethodToolCallbackPrimitiveArgumentTests.java b/spring-ai-model/src/test/java/org/springframework/ai/tool/method/MethodToolCallbackPrimitiveArgumentTests.java new file mode 100644 index 0000000000..89240c8ee4 --- /dev/null +++ b/spring-ai-model/src/test/java/org/springframework/ai/tool/method/MethodToolCallbackPrimitiveArgumentTests.java @@ -0,0 +1,101 @@ +/* + * Copyright 2023-present 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 + * + * https://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 org.springframework.ai.tool.method; + +import org.junit.jupiter.api.Test; + +import org.springframework.ai.support.ToolCallbacks; +import org.springframework.ai.tool.ToolCallback; +import org.springframework.ai.tool.annotation.Tool; +import org.springframework.ai.tool.execution.ToolExecutionException; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Regression tests for GH-6723: a missing primitive tool parameter must surface as a + * {@link ToolExecutionException} so {@code ToolExecutionExceptionProcessor} can convert + * it into a tool result the model can read and retry from. + * + * @author arimu1 + */ +class MethodToolCallbackPrimitiveArgumentTests { + + /** The model omitted "includeHourly" — a routine occurrence with smaller models. */ + private static final String MODEL_OUTPUT_OMITTING_PRIMITIVE = "{\"city\": \"Rome\"}"; + + private static ToolCallback toolNamed(String name) { + for (ToolCallback candidate : ToolCallbacks.from(new WeatherTools())) { + if (candidate.getToolDefinition().name().equals(name)) { + return candidate; + } + } + throw new AssertionError("no such tool: " + name); + } + + @Test + void wrapperParameter_acceptsMissingAsNull() { + ToolCallback tool = toolNamed("forecastBoxed"); + assertThat(tool.call(MODEL_OUTPUT_OMITTING_PRIMITIVE)).isEqualTo("\"Rome / hourly=null\""); + } + + @Test + void missingPrimitiveBoolean_throwsToolExecutionException() { + ToolCallback tool = toolNamed("forecast"); + + assertThatThrownBy(() -> tool.call(MODEL_OUTPUT_OMITTING_PRIMITIVE)).isInstanceOf(ToolExecutionException.class) + .hasMessageContaining("includeHourly") + .hasMessageContaining("boolean") + .hasCauseInstanceOf(IllegalArgumentException.class); + } + + @Test + void missingPrimitiveInt_throwsToolExecutionException() { + ToolCallback tool = toolNamed("forecastDays"); + + assertThatThrownBy(() -> tool.call(MODEL_OUTPUT_OMITTING_PRIMITIVE)).isInstanceOf(ToolExecutionException.class) + .hasMessageContaining("days") + .hasMessageContaining("int") + .hasCauseInstanceOf(IllegalArgumentException.class); + } + + @Test + void presentPrimitiveBoolean_invokesSuccessfully() { + ToolCallback tool = toolNamed("forecast"); + assertThat(tool.call("{\"city\": \"Rome\", \"includeHourly\": true}")).isEqualTo("\"Rome / hourly=true\""); + } + + static class WeatherTools { + + @Tool(description = "Get the forecast, optionally including the hourly breakdown.") + String forecast(String city, boolean includeHourly) { + return city + " / hourly=" + includeHourly; + } + + @Tool(description = "Same tool, but the flag is a wrapper instead of a primitive.") + String forecastBoxed(String city, Boolean includeHourly) { + return city + " / hourly=" + includeHourly; + } + + @Tool(description = "Forecast with a primitive int parameter.") + String forecastDays(String city, int days) { + return city + " / days=" + days; + } + + } + +}