Fix skill instruction/tool injection in system prompts
This commit is contained in:
parent
f2ffd653de
commit
3733856093
5 changed files with 181 additions and 172 deletions
|
|
@ -153,65 +153,13 @@ impl PromptSection for SkillsSection {
|
|||
}
|
||||
|
||||
fn build(&self, ctx: &PromptContext<'_>) -> Result<String> {
|
||||
if ctx.skills.is_empty() {
|
||||
return Ok(String::new());
|
||||
}
|
||||
|
||||
let mut prompt = String::from("## Available Skills\n\n<available_skills>\n");
|
||||
for skill in ctx.skills {
|
||||
let location = skill.location.clone().unwrap_or_else(|| {
|
||||
ctx.workspace_dir
|
||||
.join("skills")
|
||||
.join(&skill.name)
|
||||
.join("SKILL.md")
|
||||
});
|
||||
let escaped_name = xml_escape(&skill.name);
|
||||
let escaped_description = xml_escape(&skill.description);
|
||||
let escaped_location = xml_escape(&location.display().to_string());
|
||||
let _ = writeln!(prompt, " <skill>");
|
||||
let _ = writeln!(prompt, " <name>{escaped_name}</name>");
|
||||
let _ = writeln!(
|
||||
prompt,
|
||||
" <description>{escaped_description}</description>"
|
||||
);
|
||||
let _ = writeln!(prompt, " <location>{escaped_location}</location>");
|
||||
if !skill.tools.is_empty() {
|
||||
let _ = writeln!(prompt, " <tools>");
|
||||
for tool in &skill.tools {
|
||||
let escaped_tool_name = xml_escape(&tool.name);
|
||||
let escaped_tool_kind = xml_escape(&tool.kind);
|
||||
let escaped_tool_description = xml_escape(&tool.description);
|
||||
let _ = writeln!(
|
||||
prompt,
|
||||
" <tool name=\"{}\" kind=\"{}\">{}</tool>",
|
||||
escaped_tool_name, escaped_tool_kind, escaped_tool_description
|
||||
);
|
||||
}
|
||||
let _ = writeln!(prompt, " </tools>");
|
||||
}
|
||||
if !skill.prompts.is_empty() {
|
||||
let _ = writeln!(prompt, " <instructions>");
|
||||
for p in &skill.prompts {
|
||||
let escaped_prompt = xml_escape(p);
|
||||
let _ = writeln!(prompt, " {escaped_prompt}");
|
||||
}
|
||||
let _ = writeln!(prompt, " </instructions>");
|
||||
}
|
||||
let _ = writeln!(prompt, " </skill>");
|
||||
}
|
||||
prompt.push_str("</available_skills>");
|
||||
Ok(prompt)
|
||||
Ok(crate::skills::skills_to_prompt(
|
||||
ctx.skills,
|
||||
ctx.workspace_dir,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
fn xml_escape(raw: &str) -> String {
|
||||
raw.replace('&', "&")
|
||||
.replace('<', "<")
|
||||
.replace('>', ">")
|
||||
.replace('"', """)
|
||||
.replace('\'', "'")
|
||||
}
|
||||
|
||||
impl PromptSection for WorkspaceSection {
|
||||
fn name(&self) -> &str {
|
||||
"workspace"
|
||||
|
|
@ -383,6 +331,43 @@ mod tests {
|
|||
assert!(prompt.contains("instr"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skills_section_includes_instructions_and_tools() {
|
||||
let tools: Vec<Box<dyn Tool>> = vec![];
|
||||
let skills = vec![crate::skills::Skill {
|
||||
name: "deploy".into(),
|
||||
description: "Release safely".into(),
|
||||
version: "1.0.0".into(),
|
||||
author: None,
|
||||
tags: vec![],
|
||||
tools: vec![crate::skills::SkillTool {
|
||||
name: "release_checklist".into(),
|
||||
description: "Validate release readiness".into(),
|
||||
kind: "shell".into(),
|
||||
command: "echo ok".into(),
|
||||
args: std::collections::HashMap::new(),
|
||||
}],
|
||||
prompts: vec!["Run smoke tests before deploy.".into()],
|
||||
location: None,
|
||||
}];
|
||||
|
||||
let ctx = PromptContext {
|
||||
workspace_dir: Path::new("/tmp"),
|
||||
model_name: "test-model",
|
||||
tools: &tools,
|
||||
skills: &skills,
|
||||
identity_config: None,
|
||||
dispatcher_instructions: "",
|
||||
};
|
||||
|
||||
let output = SkillsSection.build(&ctx).unwrap();
|
||||
assert!(output.contains("<available_skills>"));
|
||||
assert!(output.contains("<name>deploy</name>"));
|
||||
assert!(output.contains("<instruction>Run smoke tests before deploy.</instruction>"));
|
||||
assert!(output.contains("<name>release_checklist</name>"));
|
||||
assert!(output.contains("<kind>shell</kind>"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn datetime_section_includes_timestamp_and_timezone() {
|
||||
let tools: Vec<Box<dyn Tool>> = vec![];
|
||||
|
|
@ -439,11 +424,11 @@ mod tests {
|
|||
assert!(prompt.contains(
|
||||
"<description>Review "unsafe" and 'risky' bits</description>"
|
||||
));
|
||||
assert!(
|
||||
prompt.contains(
|
||||
"<tool name=\"run"linter"\" kind=\"shell&exec\">Run <lint> & report</tool>"
|
||||
)
|
||||
);
|
||||
assert!(prompt.contains("Use <tool_call> and & keep output "safe""));
|
||||
assert!(prompt.contains("<name>run"linter"</name>"));
|
||||
assert!(prompt.contains("<description>Run <lint> & report</description>"));
|
||||
assert!(prompt.contains("<kind>shell&exec</kind>"));
|
||||
assert!(prompt.contains(
|
||||
"<instruction>Use <tool_call> and & keep output "safe"</instruction>"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1194,7 +1194,7 @@ fn load_openclaw_bootstrap_files(
|
|||
/// Follows the `OpenClaw` framework structure by default:
|
||||
/// 1. Tooling — tool list + descriptions
|
||||
/// 2. Safety — guardrail reminder
|
||||
/// 3. Skills — compact list with paths (loaded on-demand)
|
||||
/// 3. Skills — full skill instructions and tool metadata
|
||||
/// 4. Workspace — working directory
|
||||
/// 5. Bootstrap files — AGENTS, SOUL, TOOLS, IDENTITY, USER, BOOTSTRAP, MEMORY
|
||||
/// 6. Date & Time — timezone for cache stability
|
||||
|
|
@ -1271,52 +1271,10 @@ pub fn build_system_prompt(
|
|||
- When in doubt, ask before acting externally.\n\n",
|
||||
);
|
||||
|
||||
// ── 3. Skills (inline prompts + tools) ──────────────────────
|
||||
// ── 3. Skills (full instructions + tool metadata) ───────────
|
||||
if !skills.is_empty() {
|
||||
prompt.push_str("## Available Skills\n\n");
|
||||
prompt.push_str("<available_skills>\n");
|
||||
for skill in skills {
|
||||
let escaped_name = xml_escape(&skill.name);
|
||||
let escaped_description = xml_escape(&skill.description);
|
||||
let _ = writeln!(prompt, " <skill>");
|
||||
let _ = writeln!(prompt, " <name>{escaped_name}</name>");
|
||||
let _ = writeln!(
|
||||
prompt,
|
||||
" <description>{escaped_description}</description>"
|
||||
);
|
||||
let location = skill.location.clone().unwrap_or_else(|| {
|
||||
workspace_dir
|
||||
.join("skills")
|
||||
.join(&skill.name)
|
||||
.join("SKILL.md")
|
||||
});
|
||||
let escaped_location = xml_escape(&location.display().to_string());
|
||||
let _ = writeln!(prompt, " <location>{escaped_location}</location>");
|
||||
if !skill.tools.is_empty() {
|
||||
let _ = writeln!(prompt, " <tools>");
|
||||
for tool in &skill.tools {
|
||||
let escaped_tool_name = xml_escape(&tool.name);
|
||||
let escaped_tool_kind = xml_escape(&tool.kind);
|
||||
let escaped_tool_description = xml_escape(&tool.description);
|
||||
let _ = writeln!(
|
||||
prompt,
|
||||
" <tool name=\"{}\" kind=\"{}\">{}</tool>",
|
||||
escaped_tool_name, escaped_tool_kind, escaped_tool_description
|
||||
);
|
||||
}
|
||||
let _ = writeln!(prompt, " </tools>");
|
||||
}
|
||||
if !skill.prompts.is_empty() {
|
||||
let _ = writeln!(prompt, " <instructions>");
|
||||
for p in &skill.prompts {
|
||||
let escaped_prompt = xml_escape(p);
|
||||
let _ = writeln!(prompt, " {escaped_prompt}");
|
||||
}
|
||||
let _ = writeln!(prompt, " </instructions>");
|
||||
}
|
||||
let _ = writeln!(prompt, " </skill>");
|
||||
}
|
||||
prompt.push_str("</available_skills>\n\n");
|
||||
prompt.push_str(&crate::skills::skills_to_prompt(skills, workspace_dir));
|
||||
prompt.push_str("\n\n");
|
||||
}
|
||||
|
||||
// ── 4. Workspace ────────────────────────────────────────────
|
||||
|
|
@ -1399,14 +1357,6 @@ pub fn build_system_prompt(
|
|||
}
|
||||
}
|
||||
|
||||
fn xml_escape(raw: &str) -> String {
|
||||
raw.replace('&', "&")
|
||||
.replace('<', "<")
|
||||
.replace('>', ">")
|
||||
.replace('"', """)
|
||||
.replace('\'', "'")
|
||||
}
|
||||
|
||||
/// Inject a single workspace file into the prompt with truncation and missing-file markers.
|
||||
fn inject_workspace_file(
|
||||
prompt: &mut String,
|
||||
|
|
@ -3692,7 +3642,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn prompt_skills_inline_prompts_and_tools() {
|
||||
fn prompt_skills_include_instructions_and_tools() {
|
||||
let ws = make_workspace();
|
||||
let skills = vec![crate::skills::Skill {
|
||||
name: "code-review".into(),
|
||||
|
|
@ -3701,13 +3651,13 @@ mod tests {
|
|||
author: None,
|
||||
tags: vec![],
|
||||
tools: vec![crate::skills::SkillTool {
|
||||
name: "run_linter".into(),
|
||||
description: "Run linter on code".into(),
|
||||
name: "lint".into(),
|
||||
description: "Run static checks".into(),
|
||||
kind: "shell".into(),
|
||||
command: "cargo clippy".into(),
|
||||
args: std::collections::HashMap::new(),
|
||||
args: HashMap::new(),
|
||||
}],
|
||||
prompts: vec!["When reviewing code, check for common bugs and style issues.".into()],
|
||||
prompts: vec!["Always run cargo test before final response.".into()],
|
||||
location: None,
|
||||
}];
|
||||
|
||||
|
|
@ -3717,21 +3667,13 @@ mod tests {
|
|||
assert!(prompt.contains("<name>code-review</name>"));
|
||||
assert!(prompt.contains("<description>Review code for bugs</description>"));
|
||||
assert!(prompt.contains("SKILL.md</location>"));
|
||||
// Skill prompts should be inlined
|
||||
assert!(
|
||||
prompt.contains("When reviewing code, check for common bugs"),
|
||||
"skill prompt should be inlined in system prompt"
|
||||
);
|
||||
assert!(
|
||||
prompt.contains("<instructions>"),
|
||||
"should have instructions block"
|
||||
);
|
||||
// Skill tools should be inlined
|
||||
assert!(
|
||||
prompt.contains("run_linter"),
|
||||
"skill tool should be inlined"
|
||||
);
|
||||
assert!(prompt.contains("<tools>"), "should have tools block");
|
||||
assert!(prompt.contains("<instructions>"));
|
||||
assert!(prompt
|
||||
.contains("<instruction>Always run cargo test before final response.</instruction>"));
|
||||
assert!(prompt.contains("<tools>"));
|
||||
assert!(prompt.contains("<name>lint</name>"));
|
||||
assert!(prompt.contains("<kind>shell</kind>"));
|
||||
assert!(!prompt.contains("loaded on demand"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -3748,7 +3690,7 @@ mod tests {
|
|||
description: "Run <lint> & report".into(),
|
||||
kind: "shell&exec".into(),
|
||||
command: "cargo clippy".into(),
|
||||
args: std::collections::HashMap::new(),
|
||||
args: HashMap::new(),
|
||||
}],
|
||||
prompts: vec!["Use <tool_call> and & keep output \"safe\"".into()],
|
||||
location: None,
|
||||
|
|
@ -3760,12 +3702,12 @@ mod tests {
|
|||
assert!(prompt.contains(
|
||||
"<description>Review "unsafe" and 'risky' bits</description>"
|
||||
));
|
||||
assert!(
|
||||
prompt.contains(
|
||||
"<tool name=\"run"linter"\" kind=\"shell&exec\">Run <lint> & report</tool>"
|
||||
)
|
||||
);
|
||||
assert!(prompt.contains("Use <tool_call> and & keep output "safe""));
|
||||
assert!(prompt.contains("<name>run"linter"</name>"));
|
||||
assert!(prompt.contains("<description>Run <lint> & report</description>"));
|
||||
assert!(prompt.contains("<kind>shell&exec</kind>"));
|
||||
assert!(prompt.contains(
|
||||
"<instruction>Use <tool_call> and & keep output "safe"</instruction>"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -89,6 +89,20 @@ struct NativeToolFunctionSpec {
|
|||
parameters: serde_json::Value,
|
||||
}
|
||||
|
||||
fn parse_native_tool_spec(value: serde_json::Value) -> anyhow::Result<NativeToolSpec> {
|
||||
let spec: NativeToolSpec = serde_json::from_value(value)
|
||||
.map_err(|e| anyhow::anyhow!("Invalid OpenAI tool specification: {e}"))?;
|
||||
|
||||
if spec.kind != "function" {
|
||||
anyhow::bail!(
|
||||
"Invalid OpenAI tool specification: unsupported tool type '{}', expected 'function'",
|
||||
spec.kind
|
||||
);
|
||||
}
|
||||
|
||||
Ok(spec)
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Deserialize)]
|
||||
struct NativeToolCall {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
|
@ -372,9 +386,8 @@ impl Provider for OpenAiProvider {
|
|||
tools
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(serde_json::from_value::<NativeToolSpec>)
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map_err(|e| anyhow::anyhow!("Invalid OpenAI tool specification: {e}"))?,
|
||||
.map(parse_native_tool_spec)
|
||||
.collect::<Result<Vec<_>, _>>()?,
|
||||
)
|
||||
};
|
||||
|
||||
|
|
@ -657,7 +670,7 @@ mod tests {
|
|||
}
|
||||
}
|
||||
});
|
||||
let spec: NativeToolSpec = serde_json::from_value(json).unwrap();
|
||||
let spec = parse_native_tool_spec(json).unwrap();
|
||||
assert_eq!(spec.kind, "function");
|
||||
assert_eq!(spec.function.name, "shell");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -354,39 +354,84 @@ fn extract_description(content: &str) -> String {
|
|||
.to_string()
|
||||
}
|
||||
|
||||
/// Build a system prompt addition from all loaded skills
|
||||
pub fn skills_to_prompt(skills: &[Skill]) -> String {
|
||||
fn append_xml_escaped(out: &mut String, text: &str) {
|
||||
for ch in text.chars() {
|
||||
match ch {
|
||||
'&' => out.push_str("&"),
|
||||
'<' => out.push_str("<"),
|
||||
'>' => out.push_str(">"),
|
||||
'"' => out.push_str("""),
|
||||
'\'' => out.push_str("'"),
|
||||
_ => out.push(ch),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn write_xml_text_element(out: &mut String, indent: usize, tag: &str, value: &str) {
|
||||
for _ in 0..indent {
|
||||
out.push(' ');
|
||||
}
|
||||
out.push('<');
|
||||
out.push_str(tag);
|
||||
out.push('>');
|
||||
append_xml_escaped(out, value);
|
||||
out.push_str("</");
|
||||
out.push_str(tag);
|
||||
out.push_str(">\n");
|
||||
}
|
||||
|
||||
/// Build the "Available Skills" system prompt section with full skill instructions.
|
||||
pub fn skills_to_prompt(skills: &[Skill], workspace_dir: &Path) -> String {
|
||||
use std::fmt::Write;
|
||||
|
||||
if skills.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
|
||||
let mut prompt = String::from("\n## Active Skills\n\n");
|
||||
let mut prompt = String::from(
|
||||
"## Available Skills\n\n\
|
||||
Skill instructions and tool metadata are preloaded below.\n\
|
||||
Follow these instructions directly; do not read skill files at runtime unless the user asks.\n\n\
|
||||
<available_skills>\n",
|
||||
);
|
||||
|
||||
for skill in skills {
|
||||
let _ = writeln!(prompt, "### {} (v{})", skill.name, skill.version);
|
||||
let _ = writeln!(prompt, "{}", skill.description);
|
||||
let _ = writeln!(prompt, " <skill>");
|
||||
write_xml_text_element(&mut prompt, 4, "name", &skill.name);
|
||||
write_xml_text_element(&mut prompt, 4, "description", &skill.description);
|
||||
|
||||
let location = skill.location.clone().unwrap_or_else(|| {
|
||||
workspace_dir
|
||||
.join("skills")
|
||||
.join(&skill.name)
|
||||
.join("SKILL.md")
|
||||
});
|
||||
write_xml_text_element(&mut prompt, 4, "location", &location.display().to_string());
|
||||
|
||||
if !skill.prompts.is_empty() {
|
||||
let _ = writeln!(prompt, " <instructions>");
|
||||
for instruction in &skill.prompts {
|
||||
write_xml_text_element(&mut prompt, 6, "instruction", instruction);
|
||||
}
|
||||
let _ = writeln!(prompt, " </instructions>");
|
||||
}
|
||||
|
||||
if !skill.tools.is_empty() {
|
||||
prompt.push_str("Tools:\n");
|
||||
let _ = writeln!(prompt, " <tools>");
|
||||
for tool in &skill.tools {
|
||||
let _ = writeln!(
|
||||
prompt,
|
||||
"- **{}**: {} ({})",
|
||||
tool.name, tool.description, tool.kind
|
||||
);
|
||||
let _ = writeln!(prompt, " <tool>");
|
||||
write_xml_text_element(&mut prompt, 8, "name", &tool.name);
|
||||
write_xml_text_element(&mut prompt, 8, "description", &tool.description);
|
||||
write_xml_text_element(&mut prompt, 8, "kind", &tool.kind);
|
||||
let _ = writeln!(prompt, " </tool>");
|
||||
}
|
||||
let _ = writeln!(prompt, " </tools>");
|
||||
}
|
||||
|
||||
for p in &skill.prompts {
|
||||
prompt.push_str(p);
|
||||
prompt.push('\n');
|
||||
}
|
||||
|
||||
prompt.push('\n');
|
||||
let _ = writeln!(prompt, " </skill>");
|
||||
}
|
||||
|
||||
prompt.push_str("</available_skills>");
|
||||
prompt
|
||||
}
|
||||
|
||||
|
|
@ -683,7 +728,7 @@ command = "echo hello"
|
|||
|
||||
#[test]
|
||||
fn skills_to_prompt_empty() {
|
||||
let prompt = skills_to_prompt(&[]);
|
||||
let prompt = skills_to_prompt(&[], Path::new("/tmp"));
|
||||
assert!(prompt.is_empty());
|
||||
}
|
||||
|
||||
|
|
@ -699,9 +744,10 @@ command = "echo hello"
|
|||
prompts: vec!["Do the thing.".to_string()],
|
||||
location: None,
|
||||
}];
|
||||
let prompt = skills_to_prompt(&skills);
|
||||
assert!(prompt.contains("test"));
|
||||
assert!(prompt.contains("Do the thing"));
|
||||
let prompt = skills_to_prompt(&skills, Path::new("/tmp"));
|
||||
assert!(prompt.contains("<available_skills>"));
|
||||
assert!(prompt.contains("<name>test</name>"));
|
||||
assert!(prompt.contains("<instruction>Do the thing.</instruction>"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -889,11 +935,32 @@ description = "Bare minimum"
|
|||
prompts: vec![],
|
||||
location: None,
|
||||
}];
|
||||
let prompt = skills_to_prompt(&skills);
|
||||
let prompt = skills_to_prompt(&skills, Path::new("/tmp"));
|
||||
assert!(prompt.contains("weather"));
|
||||
assert!(prompt.contains("get_weather"));
|
||||
assert!(prompt.contains("Fetch forecast"));
|
||||
assert!(prompt.contains("shell"));
|
||||
assert!(prompt.contains("<name>get_weather</name>"));
|
||||
assert!(prompt.contains("<description>Fetch forecast</description>"));
|
||||
assert!(prompt.contains("<kind>shell</kind>"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skills_to_prompt_escapes_xml_content() {
|
||||
let skills = vec![Skill {
|
||||
name: "xml<skill>".to_string(),
|
||||
description: "A & B".to_string(),
|
||||
version: "1.0.0".to_string(),
|
||||
author: None,
|
||||
tags: vec![],
|
||||
tools: vec![],
|
||||
prompts: vec!["Use <tool> & check \"quotes\".".to_string()],
|
||||
location: None,
|
||||
}];
|
||||
|
||||
let prompt = skills_to_prompt(&skills, Path::new("/tmp"));
|
||||
assert!(prompt.contains("<name>xml<skill></name>"));
|
||||
assert!(prompt.contains("<description>A & B</description>"));
|
||||
assert!(prompt.contains(
|
||||
"<instruction>Use <tool> & check "quotes".</instruction>"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue