From d7a97aeda80823c88ad865f5fbcebdaa1e0e5780 Mon Sep 17 00:00:00 2001 From: fgbg <2508020102@qq.com> Date: Fri, 10 Apr 2026 01:15:53 +0800 Subject: [PATCH 1/3] fix issue #1158 --- .../io/agentscope/core/skill/SkillBox.java | 24 +++- .../core/skill/SkillBoxSafeSkillIdTest.java | 60 ++++++++++ .../SkillBoxWindowsNacosSourceReproTest.java | 109 ++++++++++++++++++ 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java create mode 100644 agentscope-extensions/agentscope-extensions-nacos/agentscope-extensions-nacos-skill/src/test/java/io/agentscope/core/nacos/skill/SkillBoxWindowsNacosSourceReproTest.java diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java index 33f6d007f..b6581bd66 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java @@ -38,12 +38,14 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; +import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class SkillBox implements StateModule { private static final Logger logger = LoggerFactory.getLogger(SkillBox.class); private static final String BASE64_PREFIX = "base64:"; + private static final Pattern INVALID_FILE_NAME_CHARS = Pattern.compile("[\\\\/:*?\"<>|]"); private final SkillRegistry skillRegistry = new SkillRegistry(); private final AgentSkillPromptProvider skillPromptProvider; @@ -734,7 +736,7 @@ public void uploadSkillFiles() { continue; } - Path skillDir = targetDir.resolve(skillId); + Path skillDir = targetDir.resolve(unifyToSafeSkillId(skillId)); for (String resourcePath : resourcePaths) { if (!filter.accept(resourcePath)) { @@ -777,6 +779,26 @@ public void uploadSkillFiles() { logger.info("Uploaded {} skill files to: {}", fileCount, targetDir); } + /** + * Normalizes a logical skill id to a filesystem-safe directory name. + * + *

Why this is needed: some repositories (e.g. Nacos) build source identifiers like + * {@code nacos:public}. The skill id is composed as {@code name + "_" + source}, so it may + * contain {@code ':'}. On Windows, {@code ':'} is illegal in a path segment (except drive + * letters), and using it in {@link Path#resolve(String)} can fail upload with + * {@code InvalidPathException} or cause file writes to be skipped. + * + *

This method only affects filesystem paths. It does not change the logical + * {@link AgentSkill#getSkillId()} value kept in memory. + */ + static String unifyToSafeSkillId(String skillId) { + if (skillId == null || skillId.isBlank()) { + return "skill"; + } + String safe = INVALID_FILE_NAME_CHARS.matcher(skillId).replaceAll("_"); + return safe.isBlank() ? "skill" : safe; + } + private static class DefaultSkillFileFilter implements SkillFileFilter { private final Set includeFolders; private final Set includeExtensions; diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java new file mode 100644 index 000000000..43ea5cad6 --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java @@ -0,0 +1,60 @@ +/* + * 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.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.agentscope.core.tool.Toolkit; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +@Tag("unit") +class SkillBoxSafeSkillIdTest { + + @Test + @DisplayName("Should normalize unsafe chars in skill id for filesystem usage") + void testUnifyToSafeSkillId() { + assertEquals("my_skill_nacos_public", SkillBox.unifyToSafeSkillId("my_skill_nacos:public")); + assertEquals("skill", SkillBox.unifyToSafeSkillId("")); + } + + @Test + @DisplayName("Should upload resources under normalized skill id directory") + void testUploadUsesSafeSkillId(@TempDir Path tempDir) throws IOException { + SkillBox skillBox = new SkillBox(new Toolkit()); + skillBox.codeExecution().workDir(tempDir.resolve("work").toString()).withWrite().enable(); + + Map resources = new HashMap<>(); + resources.put("scripts/main.py", "print('ok')"); + AgentSkill skill = + new AgentSkill("my_skill", "desc", "content", resources, "nacos:public"); + skillBox.registerSkill(skill); + + skillBox.uploadSkillFiles(); + + Path uploadDir = skillBox.getUploadDir(); + Path expected = uploadDir.resolve("my_skill_nacos_public/scripts/main.py"); + assertTrue(Files.exists(expected)); + } +} diff --git a/agentscope-extensions/agentscope-extensions-nacos/agentscope-extensions-nacos-skill/src/test/java/io/agentscope/core/nacos/skill/SkillBoxWindowsNacosSourceReproTest.java b/agentscope-extensions/agentscope-extensions-nacos/agentscope-extensions-nacos-skill/src/test/java/io/agentscope/core/nacos/skill/SkillBoxWindowsNacosSourceReproTest.java new file mode 100644 index 000000000..686892243 --- /dev/null +++ b/agentscope-extensions/agentscope-extensions-nacos/agentscope-extensions-nacos-skill/src/test/java/io/agentscope/core/nacos/skill/SkillBoxWindowsNacosSourceReproTest.java @@ -0,0 +1,109 @@ +/* + * 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.nacos.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; + +import com.alibaba.nacos.api.ai.AiService; +import com.alibaba.nacos.api.exception.NacosException; +import io.agentscope.core.skill.AgentSkill; +import io.agentscope.core.skill.SkillBox; +import io.agentscope.core.tool.Toolkit; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +/** + * Windows repro for Nacos source ":" issue with real {@link NacosSkillRepository} construction. + */ +@ExtendWith(MockitoExtension.class) +class SkillBoxWindowsNacosSourceReproTest { + + @Mock private AiService aiService; + + @Test + @EnabledOnOs(OS.WINDOWS) + @DisplayName("[Windows] real NacosSkillRepository source uploads via safe skill id path") + void uploadSucceedsWithRealNacosSkillRepository(@TempDir Path tempDir) + throws NacosException, IOException { + NacosSkillRepository repository = new NacosSkillRepository(aiService, "public"); + when(aiService.downloadSkillZip("repro")) + .thenReturn(createSkillZip("repro", "desc", "content", "scripts/main.py", "ok")); + + AgentSkill skill = repository.getSkill("repro"); + assertEquals("nacos:public", skill.getSource()); + + SkillBox skillBox = new SkillBox(new Toolkit()); + skillBox.registerSkill(skill); + skillBox.codeExecution() + .workDir(tempDir.resolve("work").toString()) + .withShell() + .withRead() + .withWrite() + .enable(); + + skillBox.uploadSkillFiles(); + + // SkillBox normalizes skillId for filesystem safety (':' -> '_'). + Path expected = skillBox.getUploadDir().resolve("repro_nacos_public/scripts/main.py"); + assertTrue(expected.toFile().exists(), "Resource should be uploaded under normalized path"); + } + + private static byte[] createSkillZip( + String name, + String description, + String skillContent, + String resourcePath, + String resourceContent) + throws IOException { + String root = "skill-package"; + String skillMd = + "---\n" + + "name: " + + name + + "\n" + + "description: " + + description + + "\n" + + "---\n" + + skillContent; + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ZipOutputStream zos = new ZipOutputStream(baos)) { + zos.putNextEntry(new ZipEntry(root + "/SKILL.md")); + zos.write(skillMd.getBytes()); + zos.closeEntry(); + + zos.putNextEntry(new ZipEntry(root + "/" + resourcePath)); + zos.write(resourceContent.getBytes()); + zos.closeEntry(); + + zos.finish(); + return baos.toByteArray(); + } + } +} From 38b873711ef64e8fe0dabab7d4a8c994fe72704b Mon Sep 17 00:00:00 2001 From: xuhuafei <2508020102@qq.com> Date: Fri, 10 Apr 2026 01:21:07 +0800 Subject: [PATCH 2/3] style check --- .../java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java index 43ea5cad6..1893db52e 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java @@ -47,8 +47,7 @@ void testUploadUsesSafeSkillId(@TempDir Path tempDir) throws IOException { Map resources = new HashMap<>(); resources.put("scripts/main.py", "print('ok')"); - AgentSkill skill = - new AgentSkill("my_skill", "desc", "content", resources, "nacos:public"); + AgentSkill skill = new AgentSkill("my_skill", "desc", "content", resources, "nacos:public"); skillBox.registerSkill(skill); skillBox.uploadSkillFiles(); From 79c26e99e1a258f7cb05927b66bd424da190f6de Mon Sep 17 00:00:00 2001 From: xuhuafei <2508020102@qq.com> Date: Fri, 10 Apr 2026 09:20:08 +0800 Subject: [PATCH 3/3] test cover fix --- .../io/agentscope/core/skill/SkillBox.java | 3 +- .../core/skill/SkillBoxSafeSkillIdTest.java | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java index b6581bd66..2642fc1b9 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java @@ -795,8 +795,7 @@ static String unifyToSafeSkillId(String skillId) { if (skillId == null || skillId.isBlank()) { return "skill"; } - String safe = INVALID_FILE_NAME_CHARS.matcher(skillId).replaceAll("_"); - return safe.isBlank() ? "skill" : safe; + return INVALID_FILE_NAME_CHARS.matcher(skillId).replaceAll("_"); } private static class DefaultSkillFileFilter implements SkillFileFilter { diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java index 1893db52e..9eaa1d1b9 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxSafeSkillIdTest.java @@ -39,6 +39,37 @@ void testUnifyToSafeSkillId() { assertEquals("skill", SkillBox.unifyToSafeSkillId("")); } + @Test + @DisplayName("Should return default 'skill' when skillId is null") + void testUnifyToSafeSkillIdWithNull() { + assertEquals("skill", SkillBox.unifyToSafeSkillId(null)); + } + + @Test + @DisplayName("Should return default 'skill' when skillId is blank (spaces)") + void testUnifyToSafeSkillIdWithBlankSpaces() { + assertEquals("skill", SkillBox.unifyToSafeSkillId(" ")); + } + + @Test + @DisplayName("Should return default 'skill' when skillId is blank (tabs)") + void testUnifyToSafeSkillIdWithBlankTabs() { + assertEquals("skill", SkillBox.unifyToSafeSkillId("\t\t")); + } + + @Test + @DisplayName("Should return sanitized value when result is not blank") + void testUnifyToSafeSkillIdWithNonBlankResult() { + // Single unsafe char replaced + assertEquals("my_skill", SkillBox.unifyToSafeSkillId("my:skill")); + + // Multiple unsafe chars replaced + assertEquals("skill_name_with_colon", SkillBox.unifyToSafeSkillId("skill:name:with:colon")); + + // No unsafe chars - unchanged + assertEquals("safe_skill_id", SkillBox.unifyToSafeSkillId("safe_skill_id")); + } + @Test @DisplayName("Should upload resources under normalized skill id directory") void testUploadUsesSafeSkillId(@TempDir Path tempDir) throws IOException {