fix: improve creation of global skills (#1988)

Author: tusharmath

crates/forge_domain/src/env.rs

@@ -131,6 +131,16 @@ impl Environment {
         self.base_path.join("cache")
     }
 
+    /// Returns the global skills directory path (~/forge/skills)
+    pub fn global_skills_path(&self) -> PathBuf {
+        self.base_path.join("skills")
+    }
+
+    /// Returns the project-local skills directory path (.forge/skills)
+    pub fn local_skills_path(&self) -> PathBuf {
+        self.cwd.join(".forge/skills")
+    }
+
     pub fn workspace_id(&self) -> WorkspaceId {
         let mut hasher = DefaultHasher::default();
         self.cwd.hash(&mut hasher);
@@ -190,6 +200,51 @@ mod tests {
         // Verify they are different paths
         assert_ne!(agent_path, agent_cwd_path);
     }
+
+    #[test]
+    fn test_global_skills_path() {
+        let fixture: Environment = Faker.fake();
+        let fixture = fixture.base_path(PathBuf::from("/home/user/.forge"));
+
+        let actual = fixture.global_skills_path();
+        let expected = PathBuf::from("/home/user/.forge/skills");
+
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_local_skills_path() {
+        let fixture: Environment = Faker.fake();
+        let fixture = fixture.cwd(PathBuf::from("/projects/my-app"));
+
+        let actual = fixture.local_skills_path();
+        let expected = PathBuf::from("/projects/my-app/.forge/skills");
+
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn test_skills_paths_independent() {
+        let fixture: Environment = Faker.fake();
+        let fixture = fixture
+            .cwd(PathBuf::from("/projects/my-app"))
+            .base_path(PathBuf::from("/home/user/.forge"));
+
+        let global_path = fixture.global_skills_path();
+        let local_path = fixture.local_skills_path();
+
+        let expected_global = PathBuf::from("/home/user/.forge/skills");
+        let expected_local = PathBuf::from("/projects/my-app/.forge/skills");
+
+        // Verify global path uses base_path
+        assert_eq!(global_path, expected_global);
+
+        // Verify local path uses cwd
+        assert_eq!(local_path, expected_local);
+
+        // Verify they are different paths
+        assert_ne!(global_path, local_path);
+    }
 }
 
 #[test]

crates/forge_repo/src/skill_repository.rs

@@ -2,7 +2,9 @@ use std::sync::Arc;
 
 use anyhow::Context;
 use forge_app::domain::Skill;
-use forge_app::{EnvironmentInfra, FileInfoInfra, FileReaderInfra, Walker, WalkerInfra};
+use forge_app::{
+    EnvironmentInfra, FileInfoInfra, FileReaderInfra, TemplateEngine, Walker, WalkerInfra,
+};
 use forge_domain::SkillRepository;
 use futures::future::join_all;
 use gray_matter::engine::YAML;
@@ -11,7 +13,7 @@ use serde::Deserialize;
 
 /// Repository implementation for loading skills from multiple sources:
 /// 1. Built-in skills (embedded in the application)
-/// 2. Global custom skills (from ~/.forge/skills/ directory)
+/// 2. Global custom skills (from ~/forge/skills/ directory)
 /// 3. Project-local skills (from .forge/skills/ directory in current working
 ///    directory)
 ///
@@ -24,7 +26,7 @@ use serde::Deserialize;
 ///
 /// ## Directory Resolution
 /// - **Built-in skills**: Embedded in application binary
-/// - **Global skills**: `{HOME}/.forge/skills/<skill-name>/SKILL.md`
+/// - **Global skills**: `~/forge/skills/<skill-name>/SKILL.md`
 /// - **CWD skills**: `./.forge/skills/<skill-name>/SKILL.md` (relative to
 ///   current working directory)
 ///
@@ -76,17 +78,25 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> SkillR
         skills.extend(builtin_skills);
 
         // Load global skills
-        let global_dir = env.base_path.join("skills");
+        let global_dir = env.global_skills_path();
         let global_skills = self.load_skills_from_dir(&global_dir).await?;
         skills.extend(global_skills);
 
         // Load project-local skills
-        let cwd_dir = env.cwd.join(".forge/skills");
+        let cwd_dir = env.local_skills_path();
         let cwd_skills = self.load_skills_from_dir(&cwd_dir).await?;
         skills.extend(cwd_skills);
 
         // Resolve conflicts by keeping the last occurrence (CWD > Global > Built-in)
-        Ok(resolve_skill_conflicts(skills))
+        let skills = resolve_skill_conflicts(skills);
+
+        // Render all skills with environment context
+        let rendered_skills = skills
+            .into_iter()
+            .map(|skill| self.render_skill(skill, &env))
+            .collect::<anyhow::Result<Vec<_>>>()?;
+
+        Ok(rendered_skills)
     }
 }
 
@@ -200,6 +210,26 @@ impl<I: FileInfoInfra + EnvironmentInfra + FileReaderInfra + WalkerInfra> ForgeS
 
         Ok(skills)
     }
+
+    /// Renders a skill's command field with environment context
+    ///
+    /// # Arguments
+    /// * `skill` - The skill to render
+    /// * `env` - The environment containing path information
+    ///
+    /// # Errors
+    /// Returns an error if template rendering fails
+    fn render_skill(&self, skill: Skill, env: &forge_domain::Environment) -> anyhow::Result<Skill> {
+        let skill_context = serde_json::json!({
+            "global_skills_path": env.global_skills_path().display().to_string(),
+            "local_skills_path": env.local_skills_path().display().to_string(),
+        });
+
+        let rendered_command = TemplateEngine::default()
+            .render_template(forge_domain::Template::new(&skill.command), &skill_context)?;
+
+        Ok(skill.command(rendered_command))
+    }
 }
 
 /// Private type for parsing skill YAML front matter

crates/forge_repo/src/skills/create-skill/SKILL.md

@@ -266,18 +266,51 @@ Skip this step only if the skill being developed already exists, and iteration o
 
 Verb-first naming makes it immediately clear what the skill does and follows the imperative/infinitive form used throughout the skill instructions.
 
-When creating a new skill from scratch, create the skill directory structure in `.forge/skills/<skill-name>/` from the current working directory. The structure should include:
+#### Skill Location
+
+**Default: Create project-local skills unless the user explicitly requests a global skill.**
+
+When creating a new skill from scratch, create it in the project-local skills directory:
+
+- **Project-local (default)**: `{{local_skills_path}}/<skill-name>/` - Skills are project-specific and don't affect other projects. This is the safe default for most skills.
+- **Global (only when explicitly requested)**: `{{global_skills_path}}/<skill-name>/` - Skills are available across all projects. Only use this location when the user explicitly asks for a global skill or when the skill is truly reusable across all projects.
+
+The structure should include:
 
 - `SKILL.md` with YAML frontmatter (name, description) and markdown body
 - `scripts/` directory for executable code (e.g., `process.sh`, `validate.sh`)
 - `references/` directory for documentation loaded as needed
 - `assets/` directory for files used in output
 
-Example initialization:
+Example initialization (default project-local):
+
+```bash
+mkdir -p {{local_skills_path}}/edit-pdf/{scripts,references,assets}
+cat > {{local_skills_path}}/edit-pdf/SKILL.md << 'EOF'
+---
+name: edit-pdf
+description: TODO - Describe what this skill does and when to use it
+---
+
+# Edit PDF
+
+TODO - Add skill instructions here
+EOF
+
+cat > {{local_skills_path}}/edit-pdf/scripts/rotate.sh << 'EOF'
+#!/bin/bash
+# Rotate PDF using pdftk
+pdftk "$1" cat 1-endright output "$2"
+EOF
+
+chmod +x {{local_skills_path}}/edit-pdf/scripts/rotate.sh
+```
+
+If the user explicitly requests a global skill, use this instead:
 
 ```bash
-mkdir -p .forge/skills/edit-pdf/{scripts,references,assets}
-cat > .forge/skills/edit-pdf/SKILL.md << 'EOF'
+mkdir -p {{global_skills_path}}/edit-pdf/{scripts,references,assets}
+cat > {{global_skills_path}}/edit-pdf/SKILL.md << 'EOF'
 ---
 name: edit-pdf
 description: TODO - Describe what this skill does and when to use it
@@ -288,13 +321,13 @@ description: TODO - Describe what this skill does and when to use it
 TODO - Add skill instructions here
 EOF
 
-cat > .forge/skills/edit-pdf/scripts/rotate.sh << 'EOF'
+cat > {{global_skills_path}}/edit-pdf/scripts/rotate.sh << 'EOF'
 #!/bin/bash
 # Rotate PDF using pdftk
 pdftk "$1" cat 1-endright output "$2"
 EOF
 
-chmod +x .forge/skills/edit-pdf/scripts/rotate.sh
+chmod +x {{global_skills_path}}/edit-pdf/scripts/rotate.sh
 ```
 
 After initialization, customize the generated files as needed.