Skip to content

Commit 6a6d12c

Browse files
authored
fix(SkillFileSystemHelper): Fixed an issue where an existing skill prevented other skills from being saved. (agentscope-ai#1049)
## AgentScope-Java Version 1.0.10 [The version of AgentScope-Java you are working on, e.g. 1.0.9, 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] 当技能目录已存在且强制覆盖标志为false时,现在将跳过该技能而不是返回失败状态。这样可以保存后续技能而不会因为单个技能的存在而导致其他技能无法保存。 ## 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`) - [ ] Javadoc comments are complete and follow project conventions - [ ] Related documentation has been updated (e.g. links, examples, etc.) - [x] Code is ready for review
1 parent 6688559 commit 6a6d12c

2 files changed

Lines changed: 75 additions & 2 deletions

File tree

agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ public static boolean saveSkills(Path baseDir, List<AgentSkill> skills, boolean
173173
return false;
174174
}
175175

176+
int size = skills.size();
177+
int saveCount = 0;
176178
try {
177179
for (AgentSkill skill : skills) {
178180
String skillName = skill.getName();
@@ -182,7 +184,7 @@ public static boolean saveSkills(Path baseDir, List<AgentSkill> skills, boolean
182184
if (!force) {
183185
logger.info(
184186
"Skill directory already exists and force=false: {}", skillName);
185-
return false;
187+
continue; // Skip to the next skill if force=false
186188
} else {
187189
logger.info("Overwriting existing skill directory: {}", skillName);
188190
deleteDirectory(skillDir);
@@ -215,10 +217,15 @@ public static boolean saveSkills(Path baseDir, List<AgentSkill> skills, boolean
215217
}
216218
}
217219

220+
saveCount++;
218221
logger.info("Successfully saved skill: {}", skillName);
219222
}
223+
boolean allSaved = (size == saveCount);
224+
if (!allSaved) {
225+
logger.warn("Not all skills were saved. Saved {} of {}", saveCount, size);
226+
}
220227

221-
return true;
228+
return allSaved;
222229
} catch (IOException e) {
223230
logger.error("Failed to save skills", e);
224231
throw new RuntimeException("Failed to save skills", e);

agentscope-core/src/test/java/io/agentscope/core/skill/util/SkillFileSystemHelperTest.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,72 @@ void testSaveSkills_ExistingSkill_ForceDisabled() {
152152
assertEquals("Test Skill", loaded.getDescription());
153153
}
154154

155+
@Test
156+
@DisplayName(
157+
"Should save zero skills and leave file contents unchanged when all exist and force is"
158+
+ " false")
159+
void testSaveSkills_AllExistingSkills_ForceDisabled_NoSkillsSaved() throws IOException {
160+
String originalTestSkill =
161+
Files.readString(
162+
skillsBaseDir.resolve("test-skill/SKILL.md"), StandardCharsets.UTF_8);
163+
String originalAnotherSkill =
164+
Files.readString(
165+
skillsBaseDir.resolve("another-skill/SKILL.md"), StandardCharsets.UTF_8);
166+
167+
AgentSkill skill1 = new AgentSkill("test-skill", "Updated Test", "Updated content 1", null);
168+
AgentSkill skill2 =
169+
new AgentSkill("another-skill", "Updated Another", "Updated content 2", null);
170+
171+
boolean result =
172+
SkillFileSystemHelper.saveSkills(skillsBaseDir, List.of(skill1, skill2), false);
173+
174+
// 0 out of 2 saved — no coverage at all
175+
assertFalse(result);
176+
assertEquals(
177+
originalTestSkill,
178+
Files.readString(
179+
skillsBaseDir.resolve("test-skill/SKILL.md"), StandardCharsets.UTF_8),
180+
"test-skill SKILL.md must not be modified");
181+
assertEquals(
182+
originalAnotherSkill,
183+
Files.readString(
184+
skillsBaseDir.resolve("another-skill/SKILL.md"), StandardCharsets.UTF_8),
185+
"another-skill SKILL.md must not be modified");
186+
}
187+
188+
@Test
189+
@DisplayName("Should save new skills while leaving existing ones unchanged when force is false")
190+
void testSaveSkills_MixedSkills_ForceDisabled_NewSavedExistingUnchanged() throws IOException {
191+
String originalContent =
192+
Files.readString(
193+
skillsBaseDir.resolve("test-skill/SKILL.md"), StandardCharsets.UTF_8);
194+
195+
AgentSkill existingSkill =
196+
new AgentSkill("test-skill", "Updated Description", "Updated content", null);
197+
AgentSkill newSkill = new AgentSkill("brand-new-skill", "Brand New", "New content", null);
198+
199+
boolean result =
200+
SkillFileSystemHelper.saveSkills(
201+
skillsBaseDir, List.of(existingSkill, newSkill), false);
202+
203+
// 1 out of 2 saved — not all saved
204+
assertFalse(result);
205+
206+
// existing skill must not be modified
207+
assertEquals(
208+
originalContent,
209+
Files.readString(
210+
skillsBaseDir.resolve("test-skill/SKILL.md"), StandardCharsets.UTF_8),
211+
"test-skill SKILL.md must not be modified");
212+
213+
// new skill must be saved correctly
214+
AgentSkill loaded =
215+
SkillFileSystemHelper.loadSkill(skillsBaseDir, "brand-new-skill", "source");
216+
assertEquals("brand-new-skill", loaded.getName());
217+
assertEquals("Brand New", loaded.getDescription());
218+
assertEquals("New content", loaded.getSkillContent());
219+
}
220+
155221
@Test
156222
@DisplayName("Should overwrite when skill exists and force is true")
157223
void testSaveSkills_ExistingSkill_ForceEnabled() {

0 commit comments

Comments
 (0)