This is an automated email from the ASF dual-hosted git repository.

yuluo-yx pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git


The following commit(s) were added to refs/heads/master by this push:
     new 9b31baec8a fix(ai): improve SOP execution and provider config handling 
(#4224)
9b31baec8a is described below

commit 9b31baec8acdd8a96984535c433759ff1f8548a1
Author: shown <[email protected]>
AuthorDate: Tue Jul 21 23:34:06 2026 +0800

    fix(ai): improve SOP execution and provider config handling (#4224)
    
    Signed-off-by: yuluo-yx <[email protected]>
    Co-authored-by: Tomsun28 <[email protected]>
---
 .../impl/ChatClientProviderServiceImpl.java        |  41 ++++---
 .../hertzbeat/ai/sop/engine/SopEngineImpl.java     | 110 ++++++++++---------
 .../hertzbeat/ai/sop/registry/SopToolCallback.java |  81 ++++++++++----
 .../hertzbeat/ai/sop/registry/ToolRegistry.java    |  26 +++--
 .../src/main/resources/skills/README_ZH.md         |   8 +-
 .../impl/ChatClientProviderServiceImplTest.java    |  74 +++++++++++++
 .../hertzbeat/ai/sop/engine/SopEngineImplTest.java | 115 ++++++++++++++++++++
 .../ai/sop/registry/SopToolCallbackTest.java       | 119 +++++++++++++++++++++
 .../ai/sop/registry/ToolRegistryTest.java          |  63 +++++++++++
 9 files changed, 544 insertions(+), 93 deletions(-)

diff --git 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java
 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java
index dec9bfc90c..220a95c816 100644
--- 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java
+++ 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java
@@ -48,6 +48,7 @@ import org.springframework.ai.tool.ToolCallbackProvider;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.context.ApplicationContext;
+import org.springframework.util.StringUtils;
 import reactor.core.publisher.Flux;
 
 import java.io.IOException;
@@ -69,7 +70,7 @@ public class ChatClientProviderServiceImpl implements 
ChatClientProviderService
 
     private final GeneralConfigDao generalConfigDao;
 
-    private ModelProviderConfig modelProviderConfig;
+    private volatile ModelProviderConfig modelProviderConfig;
 
 
     private final SkillRegistry skillRegistry;
@@ -78,7 +79,9 @@ public class ChatClientProviderServiceImpl implements 
ChatClientProviderService
     @Qualifier("hertzbeatTools")
     private ToolCallbackProvider toolCallbackProvider;
 
-    private boolean isConfigured = false;
+    private volatile boolean configured;
+
+    private volatile boolean configurationLoaded;
 
     @Value("classpath:/prompt/system-message.st")
     private Resource systemResource;
@@ -150,7 +153,8 @@ public class ChatClientProviderServiceImpl implements 
ChatClientProviderService
                 .replace(CONVERSATION_ID_PLACEHOLDER, 
String.valueOf(conversationId));
 
             // add extra prompt for protected model to guide it to use 
protected tools
-            if (Objects.equals(modelProviderConfig.getParticipationModel(), 
"PROTECTED")) {
+            ModelProviderConfig currentConfig = modelProviderConfig;
+            if (currentConfig != null && 
Objects.equals(currentConfig.getParticipationModel(), "PROTECTED")) {
                 Map<String, Object> metadata = new HashMap<>();
                 metadata.put("conversationId", conversationId);
                 return template + 
SystemPromptTemplate.builder().resource(extraResourceProtected).build()
@@ -202,19 +206,32 @@ public class ChatClientProviderServiceImpl implements 
ChatClientProviderService
 
     @EventListener(AiProviderConfigChangeEvent.class)
     public void onAiProviderConfigChange(AiProviderConfigChangeEvent event) {
-        GeneralConfig providerConfig = generalConfigDao.findByType("provider");
-        this.modelProviderConfig = 
JsonUtil.fromJson(providerConfig.getContent(), ModelProviderConfig.class);
+        refreshProviderConfiguration();
     }
 
     @Override
     public boolean isConfigured() {
-        if (!isConfigured) {
-            GeneralConfig providerConfig = 
generalConfigDao.findByType("provider");
-            ModelProviderConfig modelProviderConfig = 
JsonUtil.fromJson(providerConfig.getContent(),
-                ModelProviderConfig.class);
-            isConfigured = modelProviderConfig != null && 
modelProviderConfig.getApiKey() != null;
-            this.modelProviderConfig = modelProviderConfig;
+        if (!configurationLoaded) {
+            synchronized (this) {
+                if (!configurationLoaded) {
+                    refreshProviderConfiguration();
+                }
+            }
+        }
+        return configured;
+    }
+
+    /**
+     * Atomically refreshes the configuration snapshot after enabling, 
disabling, or switching providers.
+     */
+    private synchronized void refreshProviderConfiguration() {
+        GeneralConfig providerConfig = generalConfigDao.findByType("provider");
+        ModelProviderConfig refreshedConfig = null;
+        if (providerConfig != null && 
StringUtils.hasText(providerConfig.getContent())) {
+            refreshedConfig = JsonUtil.fromJson(providerConfig.getContent(), 
ModelProviderConfig.class);
         }
-        return isConfigured;
+        modelProviderConfig = refreshedConfig;
+        configured = refreshedConfig != null && 
StringUtils.hasText(refreshedConfig.getApiKey());
+        configurationLoaded = true;
     }
 }
diff --git 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java
 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java
index 44ac80da12..af38c1fdd0 100644
--- 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java
+++ 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImpl.java
@@ -41,9 +41,6 @@ import reactor.core.publisher.Flux;
 @Service
 public class SopEngineImpl implements SopEngine {
 
-    // Thread-local context for each execution
-    private static final ThreadLocal<Map<String, Object>> CONTEXT_BUS = 
ThreadLocal.withInitial(HashMap::new);
-    
     private final List<SopExecutor> executors;
 
     @Autowired
@@ -55,22 +52,12 @@ public class SopEngineImpl implements SopEngine {
     public Flux<String> execute(SopDefinition definition, Map<String, Object> 
inputParams) {
         return Flux.create(sink -> {
             try {
+                OutputConfig outputConfig = resolveOutputConfig(definition);
+                Map<String, Object> context = prepareContext(definition, 
inputParams, outputConfig);
+
                 log.info("Starting execution of SOP: {}", 
definition.getName());
                 sink.next("Starting SOP: " + definition.getName() + " (v" + 
definition.getVersion() + ")");
                 
-                // Initialize context with input parameters
-                Map<String, Object> context = CONTEXT_BUS.get();
-                context.clear();
-                context.putAll(inputParams);
-                
-                // Add language configuration to context
-                OutputConfig outputConfig = definition.getOutput();
-                if (outputConfig != null) {
-                    context.put("_language", outputConfig.getLanguageCode());
-                } else {
-                    context.put("_language", "zh");
-                }
-                
                 for (SopStep step : definition.getSteps()) {
                     sink.next("Executing step [" + step.getId() + "]: " + 
step.getType());
                     
@@ -106,10 +93,9 @@ public class SopEngineImpl implements SopEngine {
                 sink.next("SOP " + definition.getName() + " completed 
successfully.");
                 sink.complete();
             } catch (Exception e) {
-                log.error("Error executing SOP {}: {}", definition.getName(), 
e.getMessage(), e);
+                String sopName = definition == null ? "unknown" : 
definition.getName();
+                log.error("Error executing SOP {}: {}", sopName, 
e.getMessage(), e);
                 sink.error(e);
-            } finally {
-                CONTEXT_BUS.remove();
             }
         });
     }
@@ -118,40 +104,20 @@ public class SopEngineImpl implements SopEngine {
     public SopResult executeSync(SopDefinition definition, Map<String, Object> 
inputParams) {
         long startTime = System.currentTimeMillis();
         List<StepResult> stepResults = new ArrayList<>();
-        Map<String, Object> context = new HashMap<>(inputParams);
-        
-        // Apply default values for parameters that are not provided
-        if (definition.getParameters() != null) {
-            for (SopParameter param : definition.getParameters()) {
-                if (!context.containsKey(param.getName()) && 
param.getDefaultValue() != null) {
-                    context.put(param.getName(), param.getDefaultValue());
-                }
-            }
-        }
-        
-        // Get output configuration first
-        OutputConfig outputConfig = definition.getOutput();
-        if (outputConfig == null) {
-            outputConfig = OutputConfig.builder()
-                    .type("simple")
-                    .format("text")
-                    .language("zh")
-                    .build();
-        }
-        
-        // Add language configuration to context
-        context.put("_language", outputConfig.getLanguageCode());
-        
+
         SopResult.SopResultBuilder resultBuilder = SopResult.builder()
-                .sopName(definition.getName())
-                .sopVersion(definition.getVersion())
+                .sopName(definition == null ? null : definition.getName())
+                .sopVersion(definition == null ? null : 
definition.getVersion())
                 .startTime(startTime);
-        
-        resultBuilder.outputType(outputConfig.getOutputType());
-        resultBuilder.outputFormat(outputConfig.getFormat() != null ? 
outputConfig.getFormat() : "text");
-        resultBuilder.language(outputConfig.getLanguageCode());
-        
+
         try {
+            OutputConfig outputConfig = resolveOutputConfig(definition);
+            Map<String, Object> context = prepareContext(definition, 
inputParams, outputConfig);
+
+            resultBuilder.outputType(outputConfig.getOutputType());
+            resultBuilder.outputFormat(outputConfig.getFormat() != null ? 
outputConfig.getFormat() : "text");
+            resultBuilder.language(outputConfig.getLanguageCode());
+
             log.info("Starting sync execution of SOP: {}", 
definition.getName());
             
             for (SopStep step : definition.getSteps()) {
@@ -217,6 +183,50 @@ public class SopEngineImpl implements SopEngine {
             return buildFailedResult(resultBuilder, stepResults, 
e.getMessage(), startTime);
         }
     }
+
+    /**
+     * Applies defaults and validates required parameters consistently for 
every execution entry point.
+     */
+    private Map<String, Object> prepareContext(SopDefinition definition, 
Map<String, Object> inputParams,
+                                                OutputConfig outputConfig) {
+        if (definition.getSteps() == null || definition.getSteps().isEmpty()) {
+            throw new IllegalArgumentException("SOP must contain at least one 
step");
+        }
+
+        Map<String, Object> context = inputParams == null ? new HashMap<>() : 
new HashMap<>(inputParams);
+        if (definition.getParameters() != null) {
+            for (SopParameter parameter : definition.getParameters()) {
+                Object value = context.get(parameter.getName());
+                if (isMissing(value) && parameter.getDefaultValue() != null) {
+                    context.put(parameter.getName(), 
parameter.getDefaultValue());
+                    value = parameter.getDefaultValue();
+                }
+                if (parameter.isRequired() && isMissing(value)) {
+                    throw new IllegalArgumentException("Required SOP parameter 
is missing: " + parameter.getName());
+                }
+            }
+        }
+        context.put("_language", outputConfig.getLanguageCode());
+        return context;
+    }
+
+    private boolean isMissing(Object value) {
+        return value == null || value instanceof String text && text.isBlank();
+    }
+
+    private OutputConfig resolveOutputConfig(SopDefinition definition) {
+        if (definition == null) {
+            throw new IllegalArgumentException("SOP definition must not be 
null");
+        }
+        if (definition.getOutput() != null) {
+            return definition.getOutput();
+        }
+        return OutputConfig.builder()
+                .type("simple")
+                .format("text")
+                .language("zh")
+                .build();
+    }
     
     private SopResult buildFailedResult(SopResult.SopResultBuilder builder, 
             List<StepResult> stepResults, String error, long startTime) {
diff --git 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java
 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java
index 90ce21e7c2..21eb124c5f 100644
--- 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java
+++ 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallback.java
@@ -17,8 +17,16 @@
 
 package org.apache.hertzbeat.ai.sop.registry;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import org.apache.hertzbeat.ai.sop.engine.SopEngine;
 import org.apache.hertzbeat.ai.sop.model.SopDefinition;
+import org.apache.hertzbeat.ai.sop.model.SopParameter;
+import org.apache.hertzbeat.ai.sop.model.SopResult;
+import org.apache.hertzbeat.common.util.JsonUtil;
 import org.springframework.ai.tool.ToolCallback;
 import org.springframework.ai.tool.definition.ToolDefinition;
 
@@ -49,33 +57,66 @@ public class SopToolCallback implements ToolCallback {
 
     @Override
     public String call(String arguments) {
-        // TODO: Parse arguments and execute SOP
-        return "SOP " + definition.getName() + " execution started.";
+        Map<String, Object> inputParams = parseArguments(arguments);
+        SopResult result = sopEngine.executeSync(definition, inputParams);
+        return result.toAiResponse();
     }
 
     private String buildInputSchema() {
-        // Build JSON Schema for SOP parameters
-        StringBuilder schema = new StringBuilder();
-        schema.append("{\"type\":\"object\",\"properties\":{");
-        
-        if (definition.getParameters() != null && 
!definition.getParameters().isEmpty()) {
-            boolean first = true;
-            for (var param : definition.getParameters()) {
-                if (!first) {
-                    schema.append(",");
+        Map<String, Object> schema = new LinkedHashMap<>();
+        Map<String, Object> properties = new LinkedHashMap<>();
+        List<String> required = new ArrayList<>();
+
+        if (definition.getParameters() != null) {
+            for (SopParameter parameter : definition.getParameters()) {
+                Map<String, Object> property = new LinkedHashMap<>();
+                property.put("type", mapType(parameter.getType()));
+                if (parameter.getDescription() != null) {
+                    property.put("description", parameter.getDescription());
+                }
+                if (parameter.getDefaultValue() != null) {
+                    property.put("default", convertDefaultValue(parameter));
                 }
-                schema.append("\"").append(param.getName()).append("\":");
-                
schema.append("{\"type\":\"").append(mapType(param.getType())).append("\"");
-                if (param.getDescription() != null) {
-                    
schema.append(",\"description\":\"").append(param.getDescription()).append("\"");
+                properties.put(parameter.getName(), property);
+                if (parameter.isRequired()) {
+                    required.add(parameter.getName());
                 }
-                schema.append("}");
-                first = false;
             }
         }
-        
-        schema.append("}}");
-        return schema.toString();
+
+        schema.put("type", "object");
+        schema.put("properties", properties);
+        if (!required.isEmpty()) {
+            schema.put("required", required);
+        }
+        schema.put("additionalProperties", false);
+        return JsonUtil.toJson(schema);
+    }
+
+    @SuppressWarnings("unchecked")
+    private Map<String, Object> parseArguments(String arguments) {
+        if (arguments == null || arguments.isBlank()) {
+            return new HashMap<>();
+        }
+        Map<String, Object> inputParams = JsonUtil.fromJson(arguments, 
Map.class);
+        if (inputParams == null) {
+            throw new IllegalArgumentException("SOP tool arguments must be a 
valid JSON object");
+        }
+        return inputParams;
+    }
+
+    private Object convertDefaultValue(SopParameter parameter) {
+        String defaultValue = parameter.getDefaultValue();
+        try {
+            return switch (mapType(parameter.getType())) {
+                case "boolean" -> Boolean.valueOf(defaultValue);
+                case "integer" -> Long.valueOf(defaultValue);
+                case "number" -> Double.valueOf(defaultValue);
+                default -> defaultValue;
+            };
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("Invalid default value for SOP 
parameter: " + parameter.getName(), e);
+        }
     }
 
     private String mapType(String type) {
diff --git 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java
 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java
index 77f3a68bfa..bf72956ee7 100644
--- 
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java
+++ 
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistry.java
@@ -129,7 +129,7 @@ public class ToolRegistry {
      */
     public Set<String> getToolNames() {
         ensureInitialized();
-        return tools.keySet();
+        return Set.copyOf(tools.keySet());
     }
 
     /**
@@ -176,15 +176,19 @@ public class ToolRegistry {
          * Invoke this tool method with the given arguments.
          */
         public String invoke(Map<String, Object> args) {
-            try {
-                Object[] methodArgs = new Object[paramInfos.size()];
-                
-                for (int i = 0; i < paramInfos.size(); i++) {
-                    ParamInfo paramInfo = paramInfos.get(i);
-                    Object value = args.get(paramInfo.name);
-                    methodArgs[i] = convertValue(value, paramInfo.type);
+            Map<String, Object> safeArgs = args == null ? Map.of() : args;
+            Object[] methodArgs = new Object[paramInfos.size()];
+
+            for (int i = 0; i < paramInfos.size(); i++) {
+                ParamInfo paramInfo = paramInfos.get(i);
+                Object value = safeArgs.get(paramInfo.name);
+                if (paramInfo.required && isMissing(value)) {
+                    throw new IllegalArgumentException("Required tool 
parameter is missing: " + paramInfo.name);
                 }
-                
+                methodArgs[i] = convertValue(value, paramInfo.type);
+            }
+
+            try {
                 Object result = method.invoke(bean, methodArgs);
                 return result != null ? result.toString() : "";
                 
@@ -194,6 +198,10 @@ public class ToolRegistry {
             }
         }
 
+        private boolean isMissing(Object value) {
+            return value == null || value instanceof String text && 
text.isBlank();
+        }
+
         private Object convertValue(Object value, Class<?> targetType) {
             if (value == null) {
                 return null;
diff --git a/hertzbeat-ai/src/main/resources/skills/README_ZH.md 
b/hertzbeat-ai/src/main/resources/skills/README_ZH.md
index dfad433185..75ec5f8dd3 100644
--- a/hertzbeat-ai/src/main/resources/skills/README_ZH.md
+++ b/hertzbeat-ai/src/main/resources/skills/README_ZH.md
@@ -65,12 +65,16 @@ POST /api/ai/sop/execute/{skillName}/ai
 
 ```
 YAML 定义 → SkillRegistry → SopEngine → Executors → SopResult
-                                ↓
-                         ToolExecutor / LlmExecutor
+               ↓                ↓
+       SopToolCallback   ToolExecutor / LlmExecutor
                                 ↓
                          ToolRegistry(自动发现 @Tool 方法)
 ```
 
+所有执行入口都由 `SopEngine` 统一补齐默认参数并校验必填参数。技能注册为 Spring AI
+工具时,`SopToolCallback` 会根据参数定义生成 JSON Schema、解析模型传入的 JSON 对象,
+同步执行 SOP 并返回 `SopResult`;调用方不应在控制器或工具外层重复实现参数校验。
+
 ## 添加新工具
 
 只需添加 `@Tool` 注解,无需修改其他代码:
diff --git 
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java
 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java
new file mode 100644
index 0000000000..d1f5f2b4bc
--- /dev/null
+++ 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImplTest.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hertzbeat.ai.service.impl;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Proxy;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hertzbeat.base.dao.GeneralConfigDao;
+import org.apache.hertzbeat.common.entity.dto.ModelProviderConfig;
+import org.apache.hertzbeat.common.entity.manager.GeneralConfig;
+import org.apache.hertzbeat.common.support.event.AiProviderConfigChangeEvent;
+import org.apache.hertzbeat.common.util.JsonUtil;
+import org.junit.jupiter.api.Test;
+import org.springframework.context.support.StaticApplicationContext;
+
+/**
+ * Verifies that the provider configuration cache reacts to enable and disable 
events.
+ */
+class ChatClientProviderServiceImplTest {
+
+    @Test
+    void configurationChangeShouldRefreshConfiguredState() {
+        AtomicReference<GeneralConfig> currentConfig = new AtomicReference<>();
+        GeneralConfigDao configDao = configDao(currentConfig);
+        ChatClientProviderServiceImpl service = new 
ChatClientProviderServiceImpl(null, configDao, null);
+
+        assertFalse(service.isConfigured());
+
+        currentConfig.set(providerConfig("sk-test"));
+        service.onAiProviderConfigChange(changeEvent());
+        assertTrue(service.isConfigured());
+
+        currentConfig.set(providerConfig(" "));
+        service.onAiProviderConfigChange(changeEvent());
+        assertFalse(service.isConfigured());
+    }
+
+    private GeneralConfigDao configDao(AtomicReference<GeneralConfig> 
currentConfig) {
+        return (GeneralConfigDao) Proxy.newProxyInstance(
+                GeneralConfigDao.class.getClassLoader(),
+                new Class<?>[]{GeneralConfigDao.class},
+                (proxy, method, args) -> "findByType".equals(method.getName()) 
? currentConfig.get() : null);
+    }
+
+    private GeneralConfig providerConfig(String apiKey) {
+        ModelProviderConfig modelConfig = new ModelProviderConfig();
+        modelConfig.setApiKey(apiKey);
+        return GeneralConfig.builder()
+                .type("provider")
+                .content(JsonUtil.toJson(modelConfig))
+                .build();
+    }
+
+    private AiProviderConfigChangeEvent changeEvent() {
+        return new AiProviderConfigChangeEvent(new StaticApplicationContext());
+    }
+}
diff --git 
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java
 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java
new file mode 100644
index 0000000000..94b281d699
--- /dev/null
+++ 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/engine/SopEngineImplTest.java
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hertzbeat.ai.sop.engine;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hertzbeat.ai.sop.executor.SopExecutor;
+import org.apache.hertzbeat.ai.sop.model.SopDefinition;
+import org.apache.hertzbeat.ai.sop.model.SopParameter;
+import org.apache.hertzbeat.ai.sop.model.SopResult;
+import org.apache.hertzbeat.ai.sop.model.SopStep;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Verifies that all SOP execution entry points follow the same parameter 
contract.
+ */
+class SopEngineImplTest {
+
+    @Test
+    void executeSyncShouldRejectMissingRequiredParameter() {
+        SopEngineImpl engine = new SopEngineImpl(List.of(new 
RecordingExecutor()));
+
+        SopResult result = engine.executeSync(definition(requiredParameter()), 
Map.of());
+
+        assertEquals("FAILED", result.getStatus());
+        assertTrue(result.getError().contains("monitorId"));
+        assertTrue(result.getSteps().isEmpty());
+    }
+
+    @Test
+    void executeSyncShouldApplyDefaultParameter() {
+        AtomicReference<Map<String, Object>> capturedContext = new 
AtomicReference<>();
+        SopEngineImpl engine = new SopEngineImpl(List.of(new 
RecordingExecutor(capturedContext)));
+
+        SopResult result = engine.executeSync(definition(defaultParameter()), 
Map.of());
+
+        assertEquals("SUCCESS", result.getStatus());
+        assertEquals("10", capturedContext.get().get("limit"));
+        assertEquals("zh", capturedContext.get().get("_language"));
+        assertEquals("10", result.getData().get("limit"));
+    }
+
+    @Test
+    void executeStreamShouldApplyTheSameParameterRules() {
+        AtomicReference<Map<String, Object>> capturedContext = new 
AtomicReference<>();
+        SopEngineImpl engine = new SopEngineImpl(List.of(new 
RecordingExecutor(capturedContext)));
+
+        List<String> events = engine.execute(definition(defaultParameter()), 
null).collectList().block();
+
+        assertNotNull(events);
+        assertTrue(events.getLast().contains("completed successfully"));
+        assertEquals("10", capturedContext.get().get("limit"));
+    }
+
+    private SopDefinition definition(SopParameter parameter) {
+        return SopDefinition.builder()
+                .name("test-sop")
+                .version("1.0")
+                .parameters(List.of(parameter))
+                
.steps(List.of(SopStep.builder().id("query").type("tool").build()))
+                .build();
+    }
+
+    private SopParameter requiredParameter() {
+        return 
SopParameter.builder().name("monitorId").type("long").required(true).build();
+    }
+
+    private SopParameter defaultParameter() {
+        return 
SopParameter.builder().name("limit").type("integer").defaultValue("10").build();
+    }
+
+    private static final class RecordingExecutor implements SopExecutor {
+
+        private final AtomicReference<Map<String, Object>> capturedContext;
+
+        private RecordingExecutor() {
+            this(new AtomicReference<>());
+        }
+
+        private RecordingExecutor(AtomicReference<Map<String, Object>> 
capturedContext) {
+            this.capturedContext = capturedContext;
+        }
+
+        @Override
+        public boolean support(String type) {
+            return "tool".equals(type);
+        }
+
+        @Override
+        public Object execute(SopStep step, Map<String, Object> context) {
+            capturedContext.set(Map.copyOf(context));
+            return "ok";
+        }
+    }
+}
diff --git 
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java
 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java
new file mode 100644
index 0000000000..0c81533410
--- /dev/null
+++ 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/SopToolCallbackTest.java
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hertzbeat.ai.sop.registry;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hertzbeat.ai.sop.engine.SopEngine;
+import org.apache.hertzbeat.ai.sop.model.OutputType;
+import org.apache.hertzbeat.ai.sop.model.SopDefinition;
+import org.apache.hertzbeat.ai.sop.model.SopParameter;
+import org.apache.hertzbeat.ai.sop.model.SopResult;
+import org.apache.hertzbeat.common.util.JsonUtil;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+
+/**
+ * Verifies the input contract and execution behavior of a SOP exposed as a 
Spring AI tool.
+ */
+class SopToolCallbackTest {
+
+    @Test
+    @SuppressWarnings("unchecked")
+    void schemaShouldDeclareRequiredParametersAndEscapeDescriptions() {
+        SopDefinition definition = definition();
+        SopToolCallback callback = new SopToolCallback(definition, new 
RecordingEngine());
+
+        Map<String, Object> schema = 
JsonUtil.fromJson(callback.getToolDefinition().inputSchema(), Map.class);
+        Map<String, Object> properties = (Map<String, Object>) 
schema.get("properties");
+        Map<String, Object> monitorId = (Map<String, Object>) 
properties.get("monitorId");
+        Map<String, Object> limit = (Map<String, Object>) 
properties.get("limit");
+
+        assertEquals(List.of("monitorId"), schema.get("required"));
+        assertEquals(false, schema.get("additionalProperties"));
+        assertEquals("integer", monitorId.get("type"));
+        assertEquals("包含 \"引号\" 的说明", monitorId.get("description"));
+        assertEquals(10, limit.get("default"));
+    }
+
+    @Test
+    void callShouldParseArgumentsAndExecuteSop() {
+        RecordingEngine engine = new RecordingEngine();
+        SopToolCallback callback = new SopToolCallback(definition(), engine);
+
+        String response = callback.call("{\"monitorId\":42}");
+
+        assertEquals(42, engine.inputParams.get().get("monitorId"));
+        assertTrue(response.contains("SUCCESS"));
+        assertFalse(response.contains("execution started"));
+    }
+
+    @Test
+    void callShouldRejectInvalidJson() {
+        SopToolCallback callback = new SopToolCallback(definition(), new 
RecordingEngine());
+
+        assertThrows(IllegalArgumentException.class, () -> 
callback.call("not-json"));
+    }
+
+    private SopDefinition definition() {
+        SopParameter monitorId = SopParameter.builder()
+                .name("monitorId")
+                .type("long")
+                .description("包含 \"引号\" 的说明")
+                .required(true)
+                .build();
+        SopParameter limit = SopParameter.builder()
+                .name("limit")
+                .type("integer")
+                .defaultValue("10")
+                .build();
+        return SopDefinition.builder()
+                .name("diagnose")
+                .description("诊断工具")
+                .version("1.0")
+                .parameters(List.of(monitorId, limit))
+                .build();
+    }
+
+    private static final class RecordingEngine implements SopEngine {
+
+        private final AtomicReference<Map<String, Object>> inputParams = new 
AtomicReference<>();
+
+        @Override
+        public Flux<String> execute(SopDefinition definition, Map<String, 
Object> inputParams) {
+            return Flux.empty();
+        }
+
+        @Override
+        public SopResult executeSync(SopDefinition definition, Map<String, 
Object> inputParams) {
+            this.inputParams.set(inputParams);
+            return SopResult.builder()
+                    .sopName(definition.getName())
+                    .sopVersion(definition.getVersion())
+                    .status("SUCCESS")
+                    .outputType(OutputType.SIMPLE)
+                    .build();
+        }
+    }
+}
diff --git 
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java
 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java
new file mode 100644
index 0000000000..c949877762
--- /dev/null
+++ 
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/sop/registry/ToolRegistryTest.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hertzbeat.ai.sop.registry;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+import org.springframework.ai.tool.annotation.Tool;
+import org.springframework.ai.tool.annotation.ToolParam;
+
+/**
+ * Verifies parameter validation and type conversion for registered tool 
methods.
+ */
+class ToolRegistryTest {
+
+    @Test
+    void invokeShouldRejectMissingRequiredParameter() throws 
NoSuchMethodException {
+        ToolRegistry.ToolMethod toolMethod = toolMethod();
+
+        IllegalArgumentException error = 
assertThrows(IllegalArgumentException.class,
+                () -> toolMethod.invoke(Map.of()));
+
+        assertEquals("Required tool parameter is missing: monitorId", 
error.getMessage());
+    }
+
+    @Test
+    void invokeShouldConvertNumericParameter() throws NoSuchMethodException {
+        ToolRegistry.ToolMethod toolMethod = toolMethod();
+
+        assertEquals("42", toolMethod.invoke(Map.of("monitorId", "42")));
+    }
+
+    private ToolRegistry.ToolMethod toolMethod() throws NoSuchMethodException {
+        Method method = ToolFixture.class.getMethod("query", Long.class);
+        return new ToolRegistry.ToolMethod(new ToolFixture(), method, 
method.getAnnotation(Tool.class));
+    }
+
+    private static final class ToolFixture {
+
+        @Tool(name = "query", description = "测试工具")
+        public String query(@ToolParam(description = "监控 ID", required = true) 
Long monitorId) {
+            return String.valueOf(monitorId);
+        }
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


Reply via email to