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 ec15be2b5..9eaabbb16 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 @@ -15,6 +15,7 @@ */ package io.agentscope.core.skill; +import io.agentscope.core.skill.util.SkillFileSystemHelper; import io.agentscope.core.state.StateModule; import io.agentscope.core.tool.AgentTool; import io.agentscope.core.tool.ExtendedModel; @@ -670,38 +671,18 @@ public Path getUploadDir() { * @throws RuntimeException if failed to create the directory */ private Path ensureWorkDirExists() { - Path workDir; - if (this.workDir == null) { // Create temporary directory try { - workDir = Files.createTempDirectory("agentscope-code-execution-"); - - // Register shutdown hook to clean up temporary directory - Runtime.getRuntime() - .addShutdownHook( - new Thread( - () -> { - try { - deleteTempDirectory(workDir); - logger.info( - "Cleaned up temporary working directory:" - + " {}", - workDir); - } catch (IOException e) { - logger.warn( - "Failed to clean up temporary directory:" - + " {}", - e.getMessage()); - } - })); + this.workDir = Files.createTempDirectory("agentscope-code-execution-"); + + SkillFileSystemHelper.registerTempDirectoryCleanup(workDir); logger.info("Created temporary working directory: {}", workDir); } catch (IOException e) { throw new RuntimeException("Failed to create temporary working directory", e); } } else { - workDir = this.workDir; // Create directory if it doesn't exist if (!Files.exists(workDir)) { try { @@ -713,7 +694,7 @@ private Path ensureWorkDirExists() { } } - return workDir; + return this.workDir; } /** @@ -722,54 +703,21 @@ private Path ensureWorkDirExists() { * @return The upload directory path */ private Path ensureUploadDirExists() { - Path targetDir = uploadDir; - if (targetDir == null) { + if (uploadDir == null) { Path resolvedWorkDir = ensureWorkDirExists(); - targetDir = resolvedWorkDir.resolve("skills"); + uploadDir = resolvedWorkDir.resolve("skills"); } - if (!Files.exists(targetDir)) { + if (!Files.exists(uploadDir)) { try { - Files.createDirectories(targetDir); - logger.info("Created upload directory: {}", targetDir); + Files.createDirectories(uploadDir); + logger.info("Created upload directory: {}", uploadDir); } catch (IOException e) { throw new RuntimeException("Failed to create upload directory", e); } } - if (uploadDir == null) { - uploadDir = targetDir; - } - - return targetDir; - } - - /** - * Deletes the temporary working directory if it was created. - * - *

- * This method only deletes directories that were created as temporary - * directories - * by this SkillBox instance. User-specified directories are never deleted. - * - * @throws IOException if deletion fails - */ - private void deleteTempDirectory(Path temporaryWorkDir) throws IOException { - if (temporaryWorkDir != null && Files.exists(temporaryWorkDir)) { - Files.walk(temporaryWorkDir) - .sorted( - (a, b) -> - -a.compareTo( - b)) // Reverse order to delete files before directories - .forEach( - path -> { - try { - Files.delete(path); - } catch (IOException e) { - logger.warn("Failed to delete: {}", path); - } - }); - } + return uploadDir; } /** diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java b/agentscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java index 22212b11e..9319c0949 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java @@ -155,7 +155,7 @@ public AgentSkillRepositoryInfo getRepositoryInfo() { @Override public String getSource() { - return source != null ? source : "filesystem_" + buildDefaultSourceSuffix(); + return source != null ? source : "filesystem:" + buildDefaultSourceSuffix(); } private String buildDefaultSourceSuffix() { @@ -170,7 +170,7 @@ private String buildDefaultSourceSuffix() { return fileName.toString(); } - return parent.getFileName() + "/" + fileName; + return parent.getFileName() + "_" + fileName; } @Override diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/ClasspathSkillRepositoryTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/ClasspathSkillRepositoryTest.java index f1fba0508..67467d18b 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/ClasspathSkillRepositoryTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/ClasspathSkillRepositoryTest.java @@ -232,6 +232,29 @@ public URL getResource(String name) { } } + // ==================== getSource Tests ==================== + + @Test + @DisplayName("Should return default source with format: classpath:path") + void testGetSource_DefaultSource() throws IOException { + repository = new ClasspathSkillRepository("test-skills"); + assertEquals("classpath:test-skills", repository.getSource()); + } + + @Test + @DisplayName("Should return custom source when provided") + void testGetSource_CustomSource() throws IOException { + repository = new ClasspathSkillRepository("test-skills", "my-custom-source"); + assertEquals("my-custom-source", repository.getSource()); + } + + @Test + @DisplayName("Should handle trailing slash in path") + void testGetSource_TrailingSlash() throws IOException { + repository = new ClasspathSkillRepository("test-skills/"); + assertEquals("classpath:test-skills", repository.getSource()); + } + // ==================== Error Handling Tests ==================== @Test diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java index ea050497d..35a2f9a46 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java @@ -131,14 +131,52 @@ void testConstructor_RelativePath() throws IOException { // ==================== getSource Tests ==================== @Test - @DisplayName("Should return correct source format") - void testGetSource() { - String source = repository.getSource(); - assertNotNull(source); - assertTrue(source.startsWith("filesystem_")); - String expectedSuffix = - skillsBaseDir.getParent().getFileName() + "/" + skillsBaseDir.getFileName(); - assertEquals("filesystem_" + expectedSuffix, source); + @DisplayName("Should return default source with format: filesystem:parent_child") + void testGetSource_DefaultSource() { + assertTrue(repository.getSource().matches("filesystem:[^_]+_skills")); + } + + @Test + @DisplayName("Should return custom source when provided") + void testGetSource_CustomSource() { + FileSystemSkillRepository repo = + new FileSystemSkillRepository(skillsBaseDir, true, "custom-source"); + assertEquals("custom-source", repo.getSource()); + } + + @Test + @DisplayName("Should use default source when null") + void testGetSource_Null() { + FileSystemSkillRepository repo = new FileSystemSkillRepository(skillsBaseDir, true, null); + assertTrue(repo.getSource().startsWith("filesystem:")); + } + + // ==================== buildDefaultSourceSuffix Tests ==================== + + @Test + @DisplayName("Should build source: parent_child") + void testBuildSourceSuffix_TwoLevels() throws IOException { + Path dir = tempDir.resolve("parent").resolve("child"); + Files.createDirectories(dir); + assertEquals("filesystem:parent_child", new FileSystemSkillRepository(dir).getSource()); + } + + @Test + @DisplayName("Should use last two levels for deep paths") + void testBuildSourceSuffix_DeepPath() throws IOException { + Path dir = tempDir.resolve("a/b/c/d"); + Files.createDirectories(dir); + assertEquals("filesystem:c_d", new FileSystemSkillRepository(dir).getSource()); + } + + @Test + @DisplayName("Should preserve special chars in path") + void testBuildSourceSuffix_SpecialChars() throws IOException { + Path dir = tempDir.resolve("my-project").resolve("skills_v1.0"); + Files.createDirectories(dir); + assertEquals( + "filesystem:my-project_skills_v1.0", + new FileSystemSkillRepository(dir).getSource()); } // ==================== getRepositoryInfo Tests ====================