Skip to main content

harness_tools_skills/
lib.rs

1//! `skill_manage` — the LLM-facing tool that lets an agent author its own skills
2//! (create/patch/edit/delete SKILL.md). State-bearing (holds the skills dir), so
3//! constructed at wiring time like `RememberThisTool`. Risk = Destructive.
4
5use async_trait::async_trait;
6use harness_core::{Tool, ToolError, ToolResult, ToolRisk, ToolSchema, World};
7use serde_json::{Value, json};
8use std::path::PathBuf;
9
10pub struct SkillManageTool {
11    dir: PathBuf,
12    schema: ToolSchema,
13}
14
15impl SkillManageTool {
16    pub fn new(skills_dir: impl Into<PathBuf>) -> Self {
17        Self {
18            dir: skills_dir.into(),
19            schema: ToolSchema {
20                name: "skill_manage".into(),
21                description: "Author your procedural memory as reusable skills. \
22                    actions: create (write a new SKILL.md), edit (overwrite an existing one), \
23                    patch (replace old_string->new_string in a skill), delete. \
24                    A skill is a SKILL.md with YAML frontmatter (name, description) + a \
25                    markdown body of numbered steps + pitfalls. Use class-level names \
26                    (e.g. 'deploy-runbook', not 'fix-bug-1234')."
27                    .into(),
28                input: json!({
29                    "type": "object",
30                    "properties": {
31                        "action": {"type": "string", "enum": ["create", "edit", "patch", "delete"]},
32                        "name": {"type": "string", "description": "lowercase-hyphenated skill name"},
33                        "content": {"type": "string", "description": "full SKILL.md (frontmatter + body) for create/edit"},
34                        "old_string": {"type": "string", "description": "exact text to replace, for patch"},
35                        "new_string": {"type": "string", "description": "replacement text, for patch"}
36                    },
37                    "required": ["action", "name"]
38                }),
39            },
40        }
41    }
42
43    fn arg<'a>(args: &'a Value, k: &str) -> Option<&'a str> {
44        args.get(k).and_then(|v| v.as_str())
45    }
46}
47
48#[async_trait]
49impl Tool for SkillManageTool {
50    fn name(&self) -> &str {
51        &self.schema.name
52    }
53    fn schema(&self) -> &ToolSchema {
54        &self.schema
55    }
56    fn risk(&self) -> ToolRisk {
57        ToolRisk::Destructive
58    }
59
60    async fn invoke(&self, args: Value, _w: &mut World) -> Result<ToolResult, ToolError> {
61        let action = Self::arg(&args, "action").ok_or_else(|| ToolError::InvalidArgs {
62            name: "skill_manage".into(),
63            reason: "action required".into(),
64        })?;
65        let name = Self::arg(&args, "name").ok_or_else(|| ToolError::InvalidArgs {
66            name: "skill_manage".into(),
67            reason: "name required".into(),
68        })?;
69
70        // Validate the skill name up front, before any filesystem access, so
71        // EVERY action is path-safe. In particular `patch` used to `join(name)`
72        // and read the file *before* validation, letting a crafted name like
73        // `../other/skill` read outside the skills dir (an existence-probe leak
74        // in multi-tenant hosts). create/edit/delete already validate inside
75        // write_skill_md/delete_skill; this also covers patch and is harmless
76        // defense-in-depth for the rest.
77        if let Err(e) = harness_skills::validate_name(name) {
78            return Ok(ToolResult {
79                ok: false,
80                content: json!({"error": e.to_string()}),
81                trace: None,
82            });
83        }
84
85        let result: Result<Value, String> = match action {
86            "create" | "edit" => {
87                let content =
88                    Self::arg(&args, "content").ok_or_else(|| ToolError::InvalidArgs {
89                        name: "skill_manage".into(),
90                        reason: "content required for create/edit".into(),
91                    })?;
92                harness_skills::write_skill_md(&self.dir, name, content)
93                    .map(|p| json!({"action": action, "name": name, "path": p.to_string_lossy()}))
94                    .map_err(|e| e.to_string())
95            }
96            "patch" => {
97                let old = Self::arg(&args, "old_string").ok_or_else(|| ToolError::InvalidArgs {
98                    name: "skill_manage".into(),
99                    reason: "old_string required for patch".into(),
100                })?;
101                let new = Self::arg(&args, "new_string").unwrap_or("");
102                let path = self.dir.join(name).join("SKILL.md");
103                match std::fs::read_to_string(&path) {
104                    Ok(cur) => {
105                        let matches = cur.matches(old).count();
106                        if matches == 0 {
107                            Err(format!("old_string not found in {name}"))
108                        } else if matches > 1 {
109                            Err(format!(
110                                "old_string not unique in {name} ({matches} matches)"
111                            ))
112                        } else {
113                            let patched = cur.replacen(old, new, 1);
114                            harness_skills::write_skill_md(&self.dir, name, &patched)
115                                .map(|p| json!({"action": "patch", "name": name, "path": p.to_string_lossy()}))
116                                .map_err(|e| e.to_string())
117                        }
118                    }
119                    Err(e) => Err(format!("read {name}: {e}")),
120                }
121            }
122            "delete" => harness_skills::delete_skill(&self.dir, name)
123                .map(|removed| json!({"action": "delete", "name": name, "removed": removed}))
124                .map_err(|e| e.to_string()),
125            other => Err(format!("unknown action `{other}`")),
126        };
127
128        match result {
129            Ok(content) => Ok(ToolResult {
130                ok: true,
131                content,
132                trace: None,
133            }),
134            Err(reason) => Ok(ToolResult {
135                ok: false,
136                content: json!({"error": reason}),
137                trace: None,
138            }),
139        }
140    }
141}
142
143#[cfg(test)]
144mod tests {
145    use super::*;
146    use harness_context::default_world;
147
148    fn tmp() -> PathBuf {
149        let n = std::time::SystemTime::now()
150            .duration_since(std::time::UNIX_EPOCH)
151            .unwrap()
152            .as_nanos();
153        std::env::temp_dir().join(format!("harness-skillmanage-{}-{n}", std::process::id()))
154    }
155
156    const SKILL: &str =
157        "---\nname: deploy-runbook\ndescription: How to deploy.\n---\n# Deploy\n1. build\n";
158
159    #[tokio::test]
160    async fn create_patch_delete() {
161        let dir = tmp();
162        let tool = SkillManageTool::new(&dir);
163        let mut w = default_world(".");
164
165        let out = tool
166            .invoke(
167                json!({"action":"create","name":"deploy-runbook","content": SKILL}),
168                &mut w,
169            )
170            .await
171            .unwrap();
172        assert!(out.ok, "create: {:?}", out.content);
173        assert!(dir.join("deploy-runbook").join("SKILL.md").exists());
174
175        let out = tool.invoke(json!({"action":"patch","name":"deploy-runbook","old_string":"1. build","new_string":"1. build\n2. test"}), &mut w).await.unwrap();
176        assert!(out.ok, "patch: {:?}", out.content);
177        let body = std::fs::read_to_string(dir.join("deploy-runbook").join("SKILL.md")).unwrap();
178        assert!(body.contains("2. test"));
179
180        let out = tool
181            .invoke(json!({"action":"delete","name":"deploy-runbook"}), &mut w)
182            .await
183            .unwrap();
184        assert!(out.ok);
185        assert!(!dir.join("deploy-runbook").exists());
186
187        let _ = std::fs::remove_dir_all(&dir);
188    }
189
190    #[tokio::test]
191    async fn bad_name_returns_ok_false() {
192        let dir = tmp();
193        let tool = SkillManageTool::new(&dir);
194        let mut w = default_world(".");
195        let out = tool
196            .invoke(
197                json!({"action":"create","name":"Bad Name","content": SKILL}),
198                &mut w,
199            )
200            .await
201            .unwrap();
202        assert!(!out.ok);
203        let _ = std::fs::remove_dir_all(&dir);
204    }
205
206    /// `patch` must reject a path-traversal name BEFORE touching the filesystem,
207    /// so it can't read a SKILL.md outside the tool's own skills dir.
208    #[tokio::test]
209    async fn patch_rejects_traversal_name() {
210        // Plant a "victim" skill in a sibling dir of the tool's skills root.
211        let base = tmp();
212        let tool_dir = base.join("attacker");
213        let victim_dir = base.join("victim");
214        std::fs::create_dir_all(victim_dir.join("secret-skill")).unwrap();
215        std::fs::write(victim_dir.join("secret-skill").join("SKILL.md"), SKILL).unwrap();
216
217        let tool = SkillManageTool::new(&tool_dir);
218        let mut w = default_world(".");
219        // ../victim/secret-skill would escape `attacker/` into `victim/`.
220        let out = tool
221            .invoke(
222                json!({"action":"patch","name":"../victim/secret-skill",
223                       "old_string":"1. build","new_string":"x"}),
224                &mut w,
225            )
226            .await
227            .unwrap();
228        assert!(
229            !out.ok,
230            "traversal name must be rejected, got: {:?}",
231            out.content
232        );
233        // The victim file must be untouched.
234        let body =
235            std::fs::read_to_string(victim_dir.join("secret-skill").join("SKILL.md")).unwrap();
236        assert_eq!(body, SKILL);
237        let _ = std::fs::remove_dir_all(&base);
238    }
239}