From 3733856093a6b93e711957dec1ffb5aabec224b6 Mon Sep 17 00:00:00 2001 From: Chummy Date: Fri, 20 Feb 2026 01:19:43 +0800 Subject: [PATCH] Fix skill instruction/tool injection in system prompts --- docs/commands-reference.md | 2 + src/agent/prompt.rs | 109 +++++++++++++++------------------- src/channels/mod.rs | 104 ++++++++------------------------- src/providers/openai.rs | 21 +++++-- src/skills/mod.rs | 117 +++++++++++++++++++++++++++++-------- 5 files changed, 181 insertions(+), 172 deletions(-) diff --git a/docs/commands-reference.md b/docs/commands-reference.md index 04a668f..c83fd20 100644 --- a/docs/commands-reference.md +++ b/docs/commands-reference.md @@ -102,6 +102,8 @@ Runtime in-chat commands (Telegram/Discord while channel server is running): - `zeroclaw skills install ` - `zeroclaw skills remove ` +Skill manifests (`SKILL.toml`) support `prompts` and `[[tools]]`; both are injected into the agent system prompt at runtime, so the model can follow skill instructions without manually reading skill files. + ### `migrate` - `zeroclaw migrate openclaw [--source ] [--dry-run]` diff --git a/src/agent/prompt.rs b/src/agent/prompt.rs index 244dccb..457f38f 100644 --- a/src/agent/prompt.rs +++ b/src/agent/prompt.rs @@ -153,65 +153,13 @@ impl PromptSection for SkillsSection { } fn build(&self, ctx: &PromptContext<'_>) -> Result { - if ctx.skills.is_empty() { - return Ok(String::new()); - } - - let mut prompt = String::from("## Available Skills\n\n\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, " "); - let _ = writeln!(prompt, " {escaped_name}"); - let _ = writeln!( - prompt, - " {escaped_description}" - ); - let _ = writeln!(prompt, " {escaped_location}"); - if !skill.tools.is_empty() { - let _ = writeln!(prompt, " "); - 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, - " {}", - escaped_tool_name, escaped_tool_kind, escaped_tool_description - ); - } - let _ = writeln!(prompt, " "); - } - if !skill.prompts.is_empty() { - let _ = writeln!(prompt, " "); - for p in &skill.prompts { - let escaped_prompt = xml_escape(p); - let _ = writeln!(prompt, " {escaped_prompt}"); - } - let _ = writeln!(prompt, " "); - } - let _ = writeln!(prompt, " "); - } - prompt.push_str(""); - 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> = 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("")); + assert!(output.contains("deploy")); + assert!(output.contains("Run smoke tests before deploy.")); + assert!(output.contains("release_checklist")); + assert!(output.contains("shell")); + } + #[test] fn datetime_section_includes_timestamp_and_timezone() { let tools: Vec> = vec![]; @@ -439,11 +424,11 @@ mod tests { assert!(prompt.contains( "Review "unsafe" and 'risky' bits" )); - assert!( - prompt.contains( - "Run <lint> & report" - ) - ); - assert!(prompt.contains("Use <tool_call> and & keep output "safe"")); + assert!(prompt.contains("run"linter"")); + assert!(prompt.contains("Run <lint> & report")); + assert!(prompt.contains("shell&exec")); + assert!(prompt.contains( + "Use <tool_call> and & keep output "safe"" + )); } } diff --git a/src/channels/mod.rs b/src/channels/mod.rs index a9f4882..4a14d27 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -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("\n"); - for skill in skills { - let escaped_name = xml_escape(&skill.name); - let escaped_description = xml_escape(&skill.description); - let _ = writeln!(prompt, " "); - let _ = writeln!(prompt, " {escaped_name}"); - let _ = writeln!( - prompt, - " {escaped_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, " {escaped_location}"); - if !skill.tools.is_empty() { - let _ = writeln!(prompt, " "); - 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, - " {}", - escaped_tool_name, escaped_tool_kind, escaped_tool_description - ); - } - let _ = writeln!(prompt, " "); - } - if !skill.prompts.is_empty() { - let _ = writeln!(prompt, " "); - for p in &skill.prompts { - let escaped_prompt = xml_escape(p); - let _ = writeln!(prompt, " {escaped_prompt}"); - } - let _ = writeln!(prompt, " "); - } - let _ = writeln!(prompt, " "); - } - prompt.push_str("\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("code-review")); assert!(prompt.contains("Review code for bugs")); assert!(prompt.contains("SKILL.md")); - // 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(""), - "should have instructions block" - ); - // Skill tools should be inlined - assert!( - prompt.contains("run_linter"), - "skill tool should be inlined" - ); - assert!(prompt.contains(""), "should have tools block"); + assert!(prompt.contains("")); + assert!(prompt + .contains("Always run cargo test before final response.")); + assert!(prompt.contains("")); + assert!(prompt.contains("lint")); + assert!(prompt.contains("shell")); + assert!(!prompt.contains("loaded on demand")); } #[test] @@ -3748,7 +3690,7 @@ mod tests { description: "Run & report".into(), kind: "shell&exec".into(), command: "cargo clippy".into(), - args: std::collections::HashMap::new(), + args: HashMap::new(), }], prompts: vec!["Use and & keep output \"safe\"".into()], location: None, @@ -3760,12 +3702,12 @@ mod tests { assert!(prompt.contains( "Review "unsafe" and 'risky' bits" )); - assert!( - prompt.contains( - "Run <lint> & report" - ) - ); - assert!(prompt.contains("Use <tool_call> and & keep output "safe"")); + assert!(prompt.contains("run"linter"")); + assert!(prompt.contains("Run <lint> & report")); + assert!(prompt.contains("shell&exec")); + assert!(prompt.contains( + "Use <tool_call> and & keep output "safe"" + )); } #[test] diff --git a/src/providers/openai.rs b/src/providers/openai.rs index 992372a..067f943 100644 --- a/src/providers/openai.rs +++ b/src/providers/openai.rs @@ -89,6 +89,20 @@ struct NativeToolFunctionSpec { parameters: serde_json::Value, } +fn parse_native_tool_spec(value: serde_json::Value) -> anyhow::Result { + 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::) - .collect::, _>>() - .map_err(|e| anyhow::anyhow!("Invalid OpenAI tool specification: {e}"))?, + .map(parse_native_tool_spec) + .collect::, _>>()?, ) }; @@ -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"); } diff --git a/src/skills/mod.rs b/src/skills/mod.rs index 4db6cbb..4a1edd8 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -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("\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\ + \n", + ); for skill in skills { - let _ = writeln!(prompt, "### {} (v{})", skill.name, skill.version); - let _ = writeln!(prompt, "{}", skill.description); + let _ = writeln!(prompt, " "); + 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, " "); + for instruction in &skill.prompts { + write_xml_text_element(&mut prompt, 6, "instruction", instruction); + } + let _ = writeln!(prompt, " "); + } if !skill.tools.is_empty() { - prompt.push_str("Tools:\n"); + let _ = writeln!(prompt, " "); for tool in &skill.tools { - let _ = writeln!( - prompt, - "- **{}**: {} ({})", - tool.name, tool.description, tool.kind - ); + let _ = writeln!(prompt, " "); + 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, " "); } + let _ = writeln!(prompt, " "); } - for p in &skill.prompts { - prompt.push_str(p); - prompt.push('\n'); - } - - prompt.push('\n'); + let _ = writeln!(prompt, " "); } + prompt.push_str(""); 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("")); + assert!(prompt.contains("test")); + assert!(prompt.contains("Do the thing.")); } #[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("get_weather")); + assert!(prompt.contains("Fetch forecast")); + assert!(prompt.contains("shell")); + } + + #[test] + fn skills_to_prompt_escapes_xml_content() { + let skills = vec![Skill { + name: "xml".to_string(), + description: "A & B".to_string(), + version: "1.0.0".to_string(), + author: None, + tags: vec![], + tools: vec![], + prompts: vec!["Use & check \"quotes\".".to_string()], + location: None, + }]; + + let prompt = skills_to_prompt(&skills, Path::new("/tmp")); + assert!(prompt.contains("xml<skill>")); + assert!(prompt.contains("A & B")); + assert!(prompt.contains( + "Use <tool> & check "quotes"." + )); } #[test]