refactor: removed redundant skill name validation from has_skill function

src/config/install_remote.rs

@@ -5,7 +5,7 @@ use crate::mcp::{McpServer, McpServersConfig};
 use crate::utils;
 use crate::utils::IS_STDOUT_TERMINAL;
 use crate::vault::{Vault, create_vault_password_file, interpolate_secrets};
-use anyhow::{Context, Result, bail};
+use anyhow::{Context, Result, anyhow, bail};
 use indexmap::IndexMap;
 use indoc::formatdoc;
 use inquire::{Confirm, Select};
@@ -418,6 +418,26 @@ fn plan_dir_into(
         let rel = src
             .strip_prefix(src_dir)
             .expect("walk_files only returns paths under src_dir");
+
+        if category == TopCategory::Skills {
+            let skill_name = rel
+                .components()
+                .next()
+                .and_then(|c| c.as_os_str().to_str())
+                .ok_or_else(|| {
+                    anyhow!(
+                        "remote skill bundle has unparseable path component: {}",
+                        rel.display()
+                    )
+                })?;
+            paths::validate_skill_name(skill_name).with_context(|| {
+                format!(
+                    "remote skill '{skill_name}' has an invalid name \
+                     (skill names must contain only ASCII alphanumerics, '-', or '_')"
+                )
+            })?;
+        }
+
         let dst = dst_dir.join(rel);
         let kind = classify_file(&src, &dst)?;
         out.push(PlannedFile {

src/config/paths.rs

@@ -282,10 +282,6 @@ pub fn list_skills() -> Vec<String> {
 }
 
 pub fn has_skill(name: &str) -> bool {
-    if validate_skill_name(name).is_err() {
-        return false;
-    }
-
     skill_file(name).is_file()
 }
 
@@ -345,11 +341,11 @@ mod tests {
     }
 
     #[test]
-    fn has_skill_returns_false_for_invalid_names() {
+    fn has_skill_returns_false_for_missing_paths() {
-        for bad in ["", "../escape", "foo/bar", ".hidden", "with space"] {
+        for absent in ["definitely-not-installed-skill-xyz", "another-missing"] {
             assert!(
-                !has_skill(bad),
+                !has_skill(absent),
-                "has_skill({bad:?}) should be false for an invalid name"
+                "has_skill({absent:?}) should be false for a missing skill"
             );
         }
     }

src/config/request_context.rs

@@ -1231,7 +1231,8 @@ impl RequestContext {
                 if let Some(ref tool_names) = role_filter {
                     agent_functions.retain(|v| {
                         tool_names.contains(&v.name)
-                            || v.name.starts_with(SKILL_FUNCTION_PREFIX)
+                            || (!matches!(agent.skills_enabled(), Some(false))
+                                && v.name.starts_with(SKILL_FUNCTION_PREFIX))
                             || v.name.starts_with(USER_FUNCTION_PREFIX)
                     });
                 }

src/main.rs

@@ -197,14 +197,18 @@ async fn run(
         println!("{skills}");
         return Ok(());
     }
-    if cli.skill.len() == 1 && !paths::has_skill(&cli.skill[0]) {
+    if cli.skill.len() == 1 {
         let name = &cli.skill[0];
-        let app = Arc::clone(&ctx.app.config);
+        paths::validate_skill_name(name)?;
-        ctx.upsert_skill(app.as_ref(), name)?;
+        if !paths::has_skill(name) {
-        return Ok(());
+            let app = Arc::clone(&ctx.app.config);
+            ctx.upsert_skill(app.as_ref(), name)?;
+            return Ok(());
+        }
     }
     if cli.skill.len() > 1 {
         for name in &cli.skill {
+            paths::validate_skill_name(name)?;
             if !paths::has_skill(name) {
                 bail!("Skill '{name}' is not installed");
             }

src/vault/utils.rs

@@ -13,6 +13,7 @@ use gman::providers::one_password::OnePasswordProvider;
 use indoc::formatdoc;
 use inquire::validator::Validation;
 use inquire::{Confirm, Password, PasswordDisplayMode, Select, Text, min_length, required};
+use log::debug;
 use std::path::{Path, PathBuf};
 use std::process::Command;