Skip to content

Commit d895468

Browse files
authored
refactor(skill): refactor SkillBox with Skill util class (#724)
## AgentScope-Java Version [The version of AgentScope-Java you are working on, e.g. 1.0.8, check your pom.xml dependency version or run `mvn dependency:tree | grep agentscope-parent:pom`(only mac/linux)] ## Description [Please describe the background, purpose, changes made, and how to test this PR] ## Checklist Please check the following items before code is ready to be reviewed. - [x] Code has been formatted with `mvn spotless:apply` - [x] All tests are passing (`mvn test`) - [x] Javadoc comments are complete and follow project conventions - [x] Related documentation has been updated (e.g. links, examples, etc.) - [x] Code is ready for review
1 parent 6d7856c commit d895468

4 files changed

Lines changed: 82 additions & 73 deletions

File tree

agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java

Lines changed: 11 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.agentscope.core.skill;
1717

18+
import io.agentscope.core.skill.util.SkillFileSystemHelper;
1819
import io.agentscope.core.state.StateModule;
1920
import io.agentscope.core.tool.AgentTool;
2021
import io.agentscope.core.tool.ExtendedModel;
@@ -670,38 +671,18 @@ public Path getUploadDir() {
670671
* @throws RuntimeException if failed to create the directory
671672
*/
672673
private Path ensureWorkDirExists() {
673-
Path workDir;
674-
675674
if (this.workDir == null) {
676675
// Create temporary directory
677676
try {
678-
workDir = Files.createTempDirectory("agentscope-code-execution-");
679-
680-
// Register shutdown hook to clean up temporary directory
681-
Runtime.getRuntime()
682-
.addShutdownHook(
683-
new Thread(
684-
() -> {
685-
try {
686-
deleteTempDirectory(workDir);
687-
logger.info(
688-
"Cleaned up temporary working directory:"
689-
+ " {}",
690-
workDir);
691-
} catch (IOException e) {
692-
logger.warn(
693-
"Failed to clean up temporary directory:"
694-
+ " {}",
695-
e.getMessage());
696-
}
697-
}));
677+
this.workDir = Files.createTempDirectory("agentscope-code-execution-");
678+
679+
SkillFileSystemHelper.registerTempDirectoryCleanup(workDir);
698680

699681
logger.info("Created temporary working directory: {}", workDir);
700682
} catch (IOException e) {
701683
throw new RuntimeException("Failed to create temporary working directory", e);
702684
}
703685
} else {
704-
workDir = this.workDir;
705686
// Create directory if it doesn't exist
706687
if (!Files.exists(workDir)) {
707688
try {
@@ -713,7 +694,7 @@ private Path ensureWorkDirExists() {
713694
}
714695
}
715696

716-
return workDir;
697+
return this.workDir;
717698
}
718699

719700
/**
@@ -722,54 +703,21 @@ private Path ensureWorkDirExists() {
722703
* @return The upload directory path
723704
*/
724705
private Path ensureUploadDirExists() {
725-
Path targetDir = uploadDir;
726-
if (targetDir == null) {
706+
if (uploadDir == null) {
727707
Path resolvedWorkDir = ensureWorkDirExists();
728-
targetDir = resolvedWorkDir.resolve("skills");
708+
uploadDir = resolvedWorkDir.resolve("skills");
729709
}
730710

731-
if (!Files.exists(targetDir)) {
711+
if (!Files.exists(uploadDir)) {
732712
try {
733-
Files.createDirectories(targetDir);
734-
logger.info("Created upload directory: {}", targetDir);
713+
Files.createDirectories(uploadDir);
714+
logger.info("Created upload directory: {}", uploadDir);
735715
} catch (IOException e) {
736716
throw new RuntimeException("Failed to create upload directory", e);
737717
}
738718
}
739719

740-
if (uploadDir == null) {
741-
uploadDir = targetDir;
742-
}
743-
744-
return targetDir;
745-
}
746-
747-
/**
748-
* Deletes the temporary working directory if it was created.
749-
*
750-
* <p>
751-
* This method only deletes directories that were created as temporary
752-
* directories
753-
* by this SkillBox instance. User-specified directories are never deleted.
754-
*
755-
* @throws IOException if deletion fails
756-
*/
757-
private void deleteTempDirectory(Path temporaryWorkDir) throws IOException {
758-
if (temporaryWorkDir != null && Files.exists(temporaryWorkDir)) {
759-
Files.walk(temporaryWorkDir)
760-
.sorted(
761-
(a, b) ->
762-
-a.compareTo(
763-
b)) // Reverse order to delete files before directories
764-
.forEach(
765-
path -> {
766-
try {
767-
Files.delete(path);
768-
} catch (IOException e) {
769-
logger.warn("Failed to delete: {}", path);
770-
}
771-
});
772-
}
720+
return uploadDir;
773721
}
774722

775723
/**

agentscope-core/src/main/java/io/agentscope/core/skill/repository/FileSystemSkillRepository.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public AgentSkillRepositoryInfo getRepositoryInfo() {
155155

156156
@Override
157157
public String getSource() {
158-
return source != null ? source : "filesystem_" + buildDefaultSourceSuffix();
158+
return source != null ? source : "filesystem:" + buildDefaultSourceSuffix();
159159
}
160160

161161
private String buildDefaultSourceSuffix() {
@@ -170,7 +170,7 @@ private String buildDefaultSourceSuffix() {
170170
return fileName.toString();
171171
}
172172

173-
return parent.getFileName() + "/" + fileName;
173+
return parent.getFileName() + "_" + fileName;
174174
}
175175

176176
@Override

agentscope-core/src/test/java/io/agentscope/core/skill/repository/ClasspathSkillRepositoryTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,29 @@ public URL getResource(String name) {
232232
}
233233
}
234234

235+
// ==================== getSource Tests ====================
236+
237+
@Test
238+
@DisplayName("Should return default source with format: classpath:path")
239+
void testGetSource_DefaultSource() throws IOException {
240+
repository = new ClasspathSkillRepository("test-skills");
241+
assertEquals("classpath:test-skills", repository.getSource());
242+
}
243+
244+
@Test
245+
@DisplayName("Should return custom source when provided")
246+
void testGetSource_CustomSource() throws IOException {
247+
repository = new ClasspathSkillRepository("test-skills", "my-custom-source");
248+
assertEquals("my-custom-source", repository.getSource());
249+
}
250+
251+
@Test
252+
@DisplayName("Should handle trailing slash in path")
253+
void testGetSource_TrailingSlash() throws IOException {
254+
repository = new ClasspathSkillRepository("test-skills/");
255+
assertEquals("classpath:test-skills", repository.getSource());
256+
}
257+
235258
// ==================== Error Handling Tests ====================
236259

237260
@Test

agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,52 @@ void testConstructor_RelativePath() throws IOException {
131131
// ==================== getSource Tests ====================
132132

133133
@Test
134-
@DisplayName("Should return correct source format")
135-
void testGetSource() {
136-
String source = repository.getSource();
137-
assertNotNull(source);
138-
assertTrue(source.startsWith("filesystem_"));
139-
String expectedSuffix =
140-
skillsBaseDir.getParent().getFileName() + "/" + skillsBaseDir.getFileName();
141-
assertEquals("filesystem_" + expectedSuffix, source);
134+
@DisplayName("Should return default source with format: filesystem:parent_child")
135+
void testGetSource_DefaultSource() {
136+
assertTrue(repository.getSource().matches("filesystem:[^_]+_skills"));
137+
}
138+
139+
@Test
140+
@DisplayName("Should return custom source when provided")
141+
void testGetSource_CustomSource() {
142+
FileSystemSkillRepository repo =
143+
new FileSystemSkillRepository(skillsBaseDir, true, "custom-source");
144+
assertEquals("custom-source", repo.getSource());
145+
}
146+
147+
@Test
148+
@DisplayName("Should use default source when null")
149+
void testGetSource_Null() {
150+
FileSystemSkillRepository repo = new FileSystemSkillRepository(skillsBaseDir, true, null);
151+
assertTrue(repo.getSource().startsWith("filesystem:"));
152+
}
153+
154+
// ==================== buildDefaultSourceSuffix Tests ====================
155+
156+
@Test
157+
@DisplayName("Should build source: parent_child")
158+
void testBuildSourceSuffix_TwoLevels() throws IOException {
159+
Path dir = tempDir.resolve("parent").resolve("child");
160+
Files.createDirectories(dir);
161+
assertEquals("filesystem:parent_child", new FileSystemSkillRepository(dir).getSource());
162+
}
163+
164+
@Test
165+
@DisplayName("Should use last two levels for deep paths")
166+
void testBuildSourceSuffix_DeepPath() throws IOException {
167+
Path dir = tempDir.resolve("a/b/c/d");
168+
Files.createDirectories(dir);
169+
assertEquals("filesystem:c_d", new FileSystemSkillRepository(dir).getSource());
170+
}
171+
172+
@Test
173+
@DisplayName("Should preserve special chars in path")
174+
void testBuildSourceSuffix_SpecialChars() throws IOException {
175+
Path dir = tempDir.resolve("my-project").resolve("skills_v1.0");
176+
Files.createDirectories(dir);
177+
assertEquals(
178+
"filesystem:my-project_skills_v1.0",
179+
new FileSystemSkillRepository(dir).getSource());
142180
}
143181

144182
// ==================== getRepositoryInfo Tests ====================

0 commit comments

Comments
 (0)