This is an automated email from the ASF dual-hosted git repository.
Aias00 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 85dff9b36f fix(ai): handle invalid skill params and failed reports
(#4297)
85dff9b36f is described below
commit 85dff9b36f55a638c5678459d6e9c906ab1583a6
Author: shown <[email protected]>
AuthorDate: Sun Aug 2 20:19:37 2026 +0800
fix(ai): handle invalid skill params and failed reports (#4297)
Signed-off-by: yuluo-yx <[email protected]>
---
.../hertzbeat/ai/tools/impl/SkillToolsImpl.java | 14 ++-
.../ai/tools/impl/SkillToolsImplTest.java | 124 +++++++++++++++++++++
2 files changed, 134 insertions(+), 4 deletions(-)
diff --git
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImpl.java
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImpl.java
index 2e13dc699a..00b2ebe355 100644
---
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImpl.java
+++
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImpl.java
@@ -125,12 +125,16 @@ public class SkillToolsImpl implements SkillTools {
// Parse parameters
Map<String, Object> params = parseParams(paramsJson);
+ if (params == null) {
+ return "Error: Skill parameters must be a valid JSON object.";
+ }
// Validate required parameters
if (skill.getParameters() != null) {
for (SopParameter paramDef : skill.getParameters()) {
if (paramDef.isRequired()) {
- if (!params.containsKey(paramDef.getName()) ||
params.get(paramDef.getName()) == null) {
+ Object value = params.get(paramDef.getName());
+ if (value == null || value instanceof String text &&
text.isBlank()) {
return "Error: Required parameter '" +
paramDef.getName() + "' is missing. "
+ "Description: " + paramDef.getDescription();
}
@@ -143,7 +147,9 @@ public class SkillToolsImpl implements SkillTools {
SopResult result = sopEngine.executeSync(skill, params);
// Check output type
- if (result.getOutputType() == OutputType.REPORT) {
+ if ("SUCCESS".equals(result.getStatus())
+ && result.getOutputType() == OutputType.REPORT
+ && result.getContent() != null) {
// Report type: return with marker for direct display to user
log.info("Skill {} returned report-type output, marking for
direct display", skillName);
return SKILL_REPORT_MARKER + "\n" + result.getContent();
@@ -167,8 +173,8 @@ public class SkillToolsImpl implements SkillTools {
try {
return JsonUtil.fromJson(paramsJson, Map.class);
} catch (Exception e) {
- log.warn("Failed to parse params JSON: {}, returning empty map",
paramsJson);
- return new HashMap<>();
+ log.warn("Failed to parse params JSON: {}", paramsJson);
+ return null;
}
}
}
diff --git
a/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImplTest.java
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImplTest.java
new file mode 100644
index 0000000000..c921751ae6
--- /dev/null
+++
b/hertzbeat-ai/src/test/java/org/apache/hertzbeat/ai/tools/impl/SkillToolsImplTest.java
@@ -0,0 +1,124 @@
+/*
+ * 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.tools.impl;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+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.ai.sop.registry.SkillRegistry;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+/**
+ * Verifies that skill parameter errors and report execution failures return
accurate, identifiable results.
+ */
+@ExtendWith(MockitoExtension.class)
+class SkillToolsImplTest {
+
+ private static final String SKILL_NAME = "diagnose";
+
+ @Mock
+ private SkillRegistry skillRegistry;
+
+ @Mock
+ private SopEngine sopEngine;
+
+ private SkillToolsImpl skillTools;
+
+ @BeforeEach
+ void setUp() {
+ skillTools = new SkillToolsImpl(skillRegistry, sopEngine);
+ }
+
+ @Test
+ void executeSkillShouldRejectInvalidJsonObject() {
+ when(skillRegistry.getSkill(SKILL_NAME)).thenReturn(skill());
+
+ String response = skillTools.executeSkill(SKILL_NAME, "not-json");
+
+ assertEquals("Error: Skill parameters must be a valid JSON object.",
response);
+ verifyNoInteractions(sopEngine);
+ }
+
+ @Test
+ void executeSkillShouldRejectBlankRequiredParameter() {
+ when(skillRegistry.getSkill(SKILL_NAME)).thenReturn(skill());
+
+ String response = skillTools.executeSkill(SKILL_NAME,
"{\"monitorId\":\" \"}");
+
+ assertTrue(response.contains("Required parameter 'monitorId' is
missing"));
+ verifyNoInteractions(sopEngine);
+ }
+
+ @Test
+ void executeSkillShouldNotMarkFailedReportForDirectDisplay() {
+ SopDefinition skill = skill();
+ SopResult failedResult = SopResult.builder()
+ .sopName(SKILL_NAME)
+ .status("FAILED")
+ .outputType(OutputType.REPORT)
+ .error("database unavailable")
+ .build();
+ when(skillRegistry.getSkill(SKILL_NAME)).thenReturn(skill);
+ when(sopEngine.executeSync(skill, java.util.Map.of("monitorId",
1))).thenReturn(failedResult);
+
+ String response = skillTools.executeSkill(SKILL_NAME,
"{\"monitorId\":1}");
+
+ assertFalse(response.startsWith(SkillToolsImpl.SKILL_REPORT_MARKER));
+ assertTrue(response.contains("database unavailable"));
+ }
+
+ @Test
+ void executeSkillShouldMarkSuccessfulReportForDirectDisplay() {
+ SopDefinition skill = skill();
+ SopResult successResult = SopResult.builder()
+ .status("SUCCESS")
+ .outputType(OutputType.REPORT)
+ .content("diagnostic report")
+ .build();
+ when(skillRegistry.getSkill(SKILL_NAME)).thenReturn(skill);
+ when(sopEngine.executeSync(skill, java.util.Map.of("monitorId",
1))).thenReturn(successResult);
+
+ String response = skillTools.executeSkill(SKILL_NAME,
"{\"monitorId\":1}");
+
+ assertEquals(SkillToolsImpl.SKILL_REPORT_MARKER + "\ndiagnostic
report", response);
+ }
+
+ private SopDefinition skill() {
+ return SopDefinition.builder()
+ .name(SKILL_NAME)
+ .parameters(List.of(SopParameter.builder()
+ .name("monitorId")
+ .required(true)
+ .description("Monitor ID")
+ .build()))
+ .build();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
