From 8a2f3d94b59897b06acf915732fc8cc1d9c6a8bd Mon Sep 17 00:00:00 2001 From: hangweizhang Date: Sun, 7 Jun 2026 11:09:39 +0800 Subject: [PATCH] fix(model): prevent NPE when GenerateOptions is null in ChatModelBase.stream() When HarnessAgent.streamEvents() is called without explicitly setting generateOptions, the options field remains null and gets passed down to ChatModelBase.stream() and eventually to doStream() implementations. OllamaChatModel.doStream() directly called options.getToolChoice() without a null-check, causing a NullPointerException: Cannot invoke "GenerateOptions.getToolChoice()" because "options" is null The non-streaming call() path was not affected because ReActAgent.buildGenerateOptions() already falls back to GenerateOptions.builder().build() when generateOptions is null. Fix: - ChatModelBase.stream(): replace null options with GenerateOptions.builder().build() before delegating to doStream(), ensuring all subclass implementations receive non-null options - OllamaChatModel.doStream(): add defensive null-check as well, in case doStream is called through other code paths - Add unit tests verifying null options are handled safely Closes #1644 --- .../agentscope/core/model/ChatModelBase.java | 8 +- .../core/model/OllamaChatModel.java | 6 +- .../model/ChatModelBaseNullOptionsTest.java | 104 ++++++++++++++++++ 3 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 agentscope-core/src/test/java/io/agentscope/core/model/ChatModelBaseNullOptionsTest.java diff --git a/agentscope-core/src/main/java/io/agentscope/core/model/ChatModelBase.java b/agentscope-core/src/main/java/io/agentscope/core/model/ChatModelBase.java index 271bc800a6..4e37e7db07 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/model/ChatModelBase.java +++ b/agentscope-core/src/main/java/io/agentscope/core/model/ChatModelBase.java @@ -42,9 +42,15 @@ public abstract class ChatModelBase implements Model { @Override public final Flux stream( List messages, List tools, GenerateOptions options) { + GenerateOptions effectiveOptions = + options != null ? options : GenerateOptions.builder().build(); return TracerRegistry.get() .callModel( - this, messages, tools, options, () -> doStream(messages, tools, options)); + this, + messages, + tools, + effectiveOptions, + () -> doStream(messages, tools, effectiveOptions)); } /** diff --git a/agentscope-core/src/main/java/io/agentscope/core/model/OllamaChatModel.java b/agentscope-core/src/main/java/io/agentscope/core/model/OllamaChatModel.java index 6a5ea2848e..dd8706dcf5 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/model/OllamaChatModel.java +++ b/agentscope-core/src/main/java/io/agentscope/core/model/OllamaChatModel.java @@ -138,11 +138,13 @@ public ChatResponse chat(List messages, GenerateOptions options) { @Override protected Flux doStream( List messages, List tools, GenerateOptions options) { + GenerateOptions effectiveOptions = + options != null ? options : GenerateOptions.builder().build(); return streamWithHttpClient( messages, tools, - options.getToolChoice(), - OllamaOptions.fromGenerateOptions(options), + effectiveOptions.getToolChoice(), + OllamaOptions.fromGenerateOptions(effectiveOptions), true); } diff --git a/agentscope-core/src/test/java/io/agentscope/core/model/ChatModelBaseNullOptionsTest.java b/agentscope-core/src/test/java/io/agentscope/core/model/ChatModelBaseNullOptionsTest.java new file mode 100644 index 0000000000..bad0d8ede1 --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/model/ChatModelBaseNullOptionsTest.java @@ -0,0 +1,104 @@ +/* + * 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.model; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import io.agentscope.core.message.Msg; +import io.agentscope.core.message.TextBlock; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; + +/** Tests for ChatModelBase null-safety when GenerateOptions is not provided. */ +@DisplayName("ChatModelBase Null GenerateOptions Tests") +class ChatModelBaseNullOptionsTest { + + /** A minimal ChatModelBase that captures the options passed to doStream. */ + private static final class StubModel extends ChatModelBase { + + private final AtomicReference capturedOptions = new AtomicReference<>(); + + @Override + protected Flux doStream( + List messages, List tools, GenerateOptions options) { + capturedOptions.set(options); + ChatResponse response = + ChatResponse.builder() + .content(List.of(TextBlock.builder().text("ok").build())) + .build(); + return Flux.just(response); + } + + @Override + public String getModelName() { + return "stub"; + } + + GenerateOptions getCapturedOptions() { + return capturedOptions.get(); + } + } + + @Test + @DisplayName("stream() with null options should not throw NPE") + void streamWithNullOptionsShouldNotThrowNPE() { + StubModel model = new StubModel(); + + // This used to throw NPE inside OllamaChatModel.doStream() etc. + // ChatModelBase.stream() should now replace null with a default instance. + Flux flux = model.stream(List.of(), null, null); + + // Subscribe to trigger the flux + ChatResponse response = flux.blockLast(); + assertNotNull(response); + + // Verify the options passed to doStream are non-null + GenerateOptions captured = model.getCapturedOptions(); + assertNotNull(captured, "doStream should receive non-null options"); + } + + @Test + @DisplayName("stream() with null options should pass default GenerateOptions to doStream") + void streamWithNullOptionsShouldPassDefaultOptions() { + StubModel model = new StubModel(); + + model.stream(List.of(), null, null).blockLast(); + + GenerateOptions captured = model.getCapturedOptions(); + assertNotNull(captured); + // Default options should have null fields (not throwing NPE on access) + assertNull(captured.getToolChoice()); + assertNull(captured.getTemperature()); + } + + @Test + @DisplayName("stream() with non-null options should pass them through unchanged") + void streamWithNonNullOptionsShouldPassThemThrough() { + StubModel model = new StubModel(); + GenerateOptions customOptions = + GenerateOptions.builder().temperature(0.5).maxTokens(100).build(); + + model.stream(List.of(), null, customOptions).blockLast(); + + GenerateOptions captured = model.getCapturedOptions(); + assertNotNull(captured); + assertNotNull(captured.getTemperature()); + } +}