Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,13 @@ public String getDescription() {
+ "**Functionality:**\n"
+ "1. Activates the specified skill (making its tools available)\n"
+ "2. Returns the requested resource content\n"
+ " usage instructions)\n"
+ "- 'SKILL.md': The skill's markdown documentation (name, description,"
+ "- Other paths: Additional resources like scripts, configs, templates, or"
+ " data files";
+ "\n"
+ "**Path rules:**\n"
+ "- Use path=\"SKILL.md\" to load the skill's markdown documentation"
+ " (name, description, usage instructions).\n"
+ "- Use exact resource paths listed by the skill, such as"
+ " \"references/guide.md\" or \"scripts/run.py\".\n"
+ "- Do not use '.', './', the skill directory, or an absolute path.";
}

@Override
Expand All @@ -108,9 +111,11 @@ public Map<String, Object> getParameters() {
"type",
"string",
"description",
"The path to the resource file within the"
+ " skill (e.g., 'SKILL.md,"
+ " references/references.md')")),
"The exact resource path within the skill."
+ " Use 'SKILL.md' to load the"
+ " skill instructions. Do not use"
+ " '.', './', directories, or"
+ " absolute paths.")),
"required", List.of("skillId", "path"));
}

Expand Down Expand Up @@ -155,10 +160,11 @@ public Mono<ToolResultBlock> callAsync(ToolCallParam param) {
* @throws IllegalArgumentException if skill doesn't exist or resource not found
*/
private String loadSkillResourceImpl(String skillId, String path) {
AgentSkill skill = validatedActiveSkill(skillId);
AgentSkill skill = validateSkillExists(skillId);

// Special handling for SKILL.md - return the skill's markdown content
if ("SKILL.md".equals(path)) {
activateSkill(skillId);
return buildSkillMarkdownResponse(skillId, skill);
}

Expand All @@ -171,6 +177,7 @@ private String loadSkillResourceImpl(String skillId, String path) {
}

String resourceContent = resources.get(path);
activateSkill(skillId);
return buildResourceResponse(skillId, path, resourceContent);
}

Expand Down Expand Up @@ -247,13 +254,14 @@ private String buildResourceNotFoundMessage(
}

/**
* Validate skill exists and activate it and its tool group.
* Validates that a skill is registered and returns its instance.
*
* @param skillId The unique identifier of the skill
* @return The skill instance
* @throws IllegalArgumentException if skill doesn't exist
* @return The registered skill instance
* @throws IllegalArgumentException if the skill is not registered
* @throws IllegalStateException if the skill cannot be loaded after validation
*/
private AgentSkill validatedActiveSkill(String skillId) {
private AgentSkill validateSkillExists(String skillId) {
if (!skillRegistry.exists(skillId)) {
throw new IllegalArgumentException(
String.format("Skill not found: '%s'. Please check the skill ID.", skillId));
Expand All @@ -267,7 +275,10 @@ private AgentSkill validatedActiveSkill(String skillId) {
+ " error.",
skillId));
}
// Set skill as active
return skill;
}

private void activateSkill(String skillId) {
skillRegistry.setSkillActive(skillId, true);
logger.info("Activated skill: {}", skillId);

Expand All @@ -279,7 +290,5 @@ private AgentSkill validatedActiveSkill(String skillId) {
toolsGroupName,
toolkit.getToolGroup(toolsGroupName).getTools());
}

return skill;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ void testCreateLoadSkillResourceTool() {
assertEquals("load_skill_through_path", tool.getName());
assertNotNull(tool.getDescription());
assertTrue(tool.getDescription().contains("Load and activate"));
assertTrue(tool.getDescription().contains("path=\"SKILL.md\""));
assertTrue(tool.getDescription().contains("Do not use '.'"));
}

@Test
Expand All @@ -136,6 +138,12 @@ void testLoadSkillResourceToolParameters() {
@SuppressWarnings("unchecked")
Map<String, Object> skillIdParam = (Map<String, Object>) properties.get("skillId");
assertNotNull(skillIdParam.get("enum"));

@SuppressWarnings("unchecked")
Map<String, Object> pathParam = (Map<String, Object>) properties.get("path");
String pathDescription = (String) pathParam.get("description");
assertTrue(pathDescription.contains("Use 'SKILL.md'"));
assertTrue(pathDescription.contains("Do not use '.'"));
}

@Test
Expand Down Expand Up @@ -483,4 +491,28 @@ void testLoadSkillWithNoToolsDoesNotFail() {
assertNull(
toolkit.getToolGroup(skillId + "_skill_tools"), "Tool group should not be created");
}

@Test
@DisplayName("Should not activate skill when resource path is invalid")
void testInvalidResourcePathDoesNotActivateSkill() {
AgentTool tool = toolkit.getTool("load_skill_through_path");
String skillId = "test_skill_custom";
assertFalse(skillBox.isSkillActive(skillId));

Map<String, Object> input = Map.of("skillId", skillId, "path", ".");
ToolUseBlock toolUseBlock =
ToolUseBlock.builder()
.id("test-call-016")
.name("load_skill_through_path")
.input(input)
.build();
ToolCallParam param =
ToolCallParam.builder().toolUseBlock(toolUseBlock).input(input).build();

ToolResultBlock result = tool.callAsync(param).block(TIMEOUT);

assertNotNull(result);
assertTrue(isErrorResult(result));
assertFalse(skillBox.isSkillActive(skillId));
}
}
Loading