This is an automated email from the ASF dual-hosted git repository. yuluo-yx pushed a commit to branch 0802-yuluo/fix in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
commit 0aebf5f8ea477b6f21af7e54c32b194a7b2be4a8 Author: yuluo-yx <[email protected]> AuthorDate: Sun Aug 2 18:43:36 2026 +0800 fix(ai): handle invalid skill params and failed reports 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..2aa9d7ac70 --- /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; + +/** + * 验证技能参数错误和报告执行失败时能够返回真实、可识别的结果。 + */ +@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]
