feat: validate visible_skills field at config load time

src/config/app_config.rs

@@ -3,7 +3,7 @@ use crate::render::{MarkdownRender, RenderOptions};
 use crate::utils::{IS_STDOUT_TERMINAL, NO_COLOR, decode_bin, get_env_name};
 
 use super::paths;
-use anyhow::{Context, Result, anyhow};
+use anyhow::{Context, Result, anyhow, bail};
 use gman::providers::SupportedProvider;
 use indexmap::IndexMap;
 use serde::Deserialize;
@@ -216,6 +216,7 @@ impl AppConfig {
             clients: config.clients,
         };
         app_config.load_envs();
+        app_config.validate_visible_skills()?;
         if let Some(wrap) = app_config.wrap.clone() {
             app_config.set_wrap(&wrap)?;
         }
@@ -225,11 +226,28 @@ impl AppConfig {
         Ok(app_config)
     }
 
+    fn validate_visible_skills(&self) -> Result<()> {
+        let Some(skills) = self.visible_skills.as_ref() else {
+            return Ok(());
+        };
+
+        for name in skills {
+            paths::validate_skill_name(name)
+                .map_err(|e| anyhow!("invalid entry in visible_skills: {e}"))?;
+
+            if !paths::has_skill(name) {
+                bail!("visible_skills references skill '{name}' which is not installed");
+            }
+        }
+
+        Ok(())
+    }
+
     pub fn resolve_model(&mut self) -> Result<()> {
         if self.model_id.is_empty() {
             let models = list_models(self, crate::client::ModelType::Chat);
             if models.is_empty() {
-                anyhow::bail!("No available model");
+                bail!("No available model");
             }
             self.model_id = models[0].id();
         }

src/config/paths.rs

@@ -289,17 +289,6 @@ pub fn has_skill(name: &str) -> bool {
     skill_file(name).is_file()
 }
 
-pub fn list_visible_skills(visible: Option<&[String]>) -> Vec<String> {
-    let installed = list_skills();
-    match visible {
-        None => installed,
-        Some(allow) => installed
-            .into_iter()
-            .filter(|name| allow.iter().any(|v| v == name))
-            .collect(),
-    }
-}
-
 pub fn local_models_override() -> Result<Vec<ProviderModels>> {
     let model_override_path = models_override_file();
     let err = || {

src/config/request_context.rs

@@ -1738,14 +1738,22 @@ impl RequestContext {
                 let raw: Option<String> = super::parse_value(value)?;
                 let parsed: Option<Vec<String>> = raw.map(|s| super::csv_to_vec(&s));
                 if let Some(names) = parsed.as_ref() {
-                    let visible =
-                        paths::list_visible_skills(self.app.config.visible_skills.as_deref());
+                    let visible = self.app.config.visible_skills.as_deref();
                     for name in names {
                         paths::validate_skill_name(name)?;
-                        if !visible.iter().any(|s| s == name) {
-                            bail!(
-                                "enabled_skills references skill '{name}' which is not visible (check global 'visible_skills' and that the skill is installed)"
-                            );
+                        match visible {
+                            Some(vs) => {
+                                if !vs.iter().any(|s| s == name) {
+                                    bail!(
+                                        "skill '{name}' is not in the global 'visible_skills' allow-list"
+                                    );
+                                }
+                            }
+                            None => {
+                                if !paths::has_skill(name) {
+                                    bail!("skill '{name}' is not installed");
+                                }
+                            }
                         }
                     }
                 }

src/config/skill_policy.rs

@@ -4,7 +4,7 @@ use super::paths;
 use super::role::Role;
 use super::session::Session;
 
-use anyhow::{Result, bail};
+use anyhow::{Result, anyhow, bail};
 use std::collections::HashSet;
 
 #[derive(Debug)]
@@ -76,16 +76,24 @@ impl SkillPolicy {
             Some(explicit) => {
                 let set: HashSet<String> = explicit.into_iter().collect();
                 for name in &set {
-                    if !skill_exists(name) {
-                        bail!("enabled_skills references skill '{name}' which is not installed");
-                    }
-
-                    if let Some(vs) = &visible
-                        && !vs.contains(name)
-                    {
-                        bail!(
-                            "enabled_skills references skill '{name}' which is not in visible_skills"
-                        );
+                    paths::validate_skill_name(name).map_err(|e| {
+                        anyhow!("enabled_skills contains invalid name '{name}': {e}")
+                    })?;
+                    match &visible {
+                        Some(vs) => {
+                            if !vs.contains(name) {
+                                bail!(
+                                    "enabled_skills references skill '{name}' which is not in the global 'visible_skills' allow-list"
+                                );
+                            }
+                        }
+                        None => {
+                            if !skill_exists(name) {
+                                bail!(
+                                    "enabled_skills references skill '{name}' which is not installed"
+                                );
+                            }
+                        }
                     }
                 }
                 set