Fix embedded skill loading: stop XML-escaping location paths
The <location> field in the skills XML prompt was being XML-escaped, converting <embedded:research>/SKILL.md to <embedded:research>/SKILL.md. When the LLM tried to use read_file with this escaped path, it would fail. Changes: - Remove escape_xml() call from location field in prompt.rs - Add fallback handling for escaped paths in try_read_embedded_skill() - Add tests for both prompt generation and read_file handling Fixes embedded skill loading for agents like butler running outside the g3 repo.
This commit is contained in:
@@ -24,7 +24,9 @@ pub fn generate_skills_prompt(skills: &[Skill]) -> String {
|
||||
xml.push_str(" <skill>\n");
|
||||
xml.push_str(&format!(" <name>{}</name>\n", escape_xml(&skill.name)));
|
||||
xml.push_str(&format!(" <description>{}</description>\n", escape_xml(&skill.description)));
|
||||
xml.push_str(&format!(" <location>{}</location>\n", escape_xml(&skill.path)));
|
||||
// Don't escape location - it's a path the LLM needs to use with read_file
|
||||
// Embedded paths like <embedded:name>/SKILL.md must remain unescaped
|
||||
xml.push_str(&format!(" <location>{}</location>\n", &skill.path));
|
||||
|
||||
// Include compatibility info if present
|
||||
if let Some(ref compat) = skill.compatibility {
|
||||
@@ -137,4 +139,20 @@ mod tests {
|
||||
assert!(result.contains("# Available Skills"));
|
||||
assert!(result.contains("read the full skill file using `read_file`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_embedded_skill_path_not_escaped() {
|
||||
// Embedded skill paths use <embedded:name> syntax which must NOT be escaped
|
||||
// so the LLM can use them directly with read_file
|
||||
let skills = vec![
|
||||
make_skill("research", "Web research skill", "<embedded:research>/SKILL.md"),
|
||||
];
|
||||
|
||||
let result = generate_skills_prompt(&skills);
|
||||
|
||||
// The path should appear unescaped
|
||||
assert!(result.contains("<location><embedded:research>/SKILL.md</location>"));
|
||||
// Should NOT be escaped
|
||||
assert!(!result.contains("<embedded:research>"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -74,17 +74,24 @@ fn calculate_read_limit(file_bytes: usize, total_tokens: u32, used_tokens: u32)
|
||||
/// Recognizes paths like:
|
||||
/// - `<embedded:research>/SKILL.md`
|
||||
/// - `<embedded:skill-name>/SKILL.md`
|
||||
/// - `<embedded:research>/SKILL.md` (XML-escaped fallback)
|
||||
///
|
||||
/// Returns the skill content if found, None otherwise.
|
||||
fn try_read_embedded_skill(path: &str) -> Option<&'static str> {
|
||||
// Check for embedded skill path pattern: <embedded:name>/SKILL.md
|
||||
if !path.starts_with("<embedded:") {
|
||||
// Also handle XML-escaped version: <embedded:name>/SKILL.md
|
||||
let (after_prefix, closing) = if path.starts_with("<embedded:") {
|
||||
// Normal unescaped path
|
||||
(path.strip_prefix("<embedded:")?, ">")
|
||||
} else if path.starts_with("<embedded:") {
|
||||
// XML-escaped path (fallback for older prompts or LLM quirks)
|
||||
(path.strip_prefix("<embedded:")?, ">")
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
};
|
||||
|
||||
// Extract skill name from path like "<embedded:research>/SKILL.md"
|
||||
let after_prefix = path.strip_prefix("<embedded:")?;
|
||||
let skill_name = after_prefix.split('>').next()?;
|
||||
// Extract skill name from path
|
||||
let skill_name = after_prefix.split(closing).next()?;
|
||||
|
||||
// Look up the embedded skill
|
||||
let skill = get_embedded_skill(skill_name)?;
|
||||
@@ -899,3 +906,41 @@ pub fn print_imgcat(
|
||||
// Blank line before next image (no │ prefix)
|
||||
println!();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_try_read_embedded_skill_unescaped() {
|
||||
// Normal unescaped path should work
|
||||
let result = try_read_embedded_skill("<embedded:research>/SKILL.md");
|
||||
assert!(result.is_some(), "Should find embedded research skill");
|
||||
assert!(result.unwrap().contains("name: research"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_read_embedded_skill_escaped() {
|
||||
// XML-escaped path should also work (fallback for LLM quirks)
|
||||
let result = try_read_embedded_skill("<embedded:research>/SKILL.md");
|
||||
assert!(result.is_some(), "Should find embedded research skill with escaped path");
|
||||
assert!(result.unwrap().contains("name: research"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_read_embedded_skill_invalid_name() {
|
||||
// Invalid skill name should return None
|
||||
let result = try_read_embedded_skill("<embedded:nonexistent>/SKILL.md");
|
||||
assert!(result.is_none(), "Should not find nonexistent skill");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_read_embedded_skill_not_embedded_path() {
|
||||
// Regular file paths should return None
|
||||
let result = try_read_embedded_skill("/path/to/SKILL.md");
|
||||
assert!(result.is_none(), "Regular path should not match");
|
||||
|
||||
let result = try_read_embedded_skill("skills/research/SKILL.md");
|
||||
assert!(result.is_none(), "Relative path should not match");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user