test: improve vault password file errors by propagating up

src/cli/completer.rs

@@ -137,13 +137,16 @@ pub(super) fn session_completer(current: &OsStr) -> Vec<CompletionCandidate> {
 pub(super) fn secrets_completer(current: &OsStr) -> Vec<CompletionCandidate> {
     let cur = current.to_string_lossy();
     match load_app_config_for_completion() {
-        Ok(app_config) => Vault::init(&app_config)
-            .list_secrets(false)
-            .unwrap_or_default()
-            .into_iter()
-            .filter(|s| s.starts_with(&*cur))
-            .map(CompletionCandidate::new)
-            .collect(),
+        Ok(app_config) => match Vault::init(&app_config) {
+            Ok(vault) => vault
+                .list_secrets(false)
+                .unwrap_or_default()
+                .into_iter()
+                .filter(|s| s.starts_with(&*cur))
+                .map(CompletionCandidate::new)
+                .collect(),
+            Err(_) => vec![],
+        },
         Err(_) => vec![],
     }
 }

src/cli/mod.rs

@@ -10,6 +10,7 @@ use clap::ValueHint;
 use clap::{Parser, crate_authors, crate_description, crate_version};
 use clap_complete::ArgValueCompleter;
 use is_terminal::IsTerminal;
+use std::collections::HashSet;
 use std::io::{Read, stdin};
 
 #[derive(Parser, Debug)]
@@ -163,6 +164,18 @@ pub struct Cli {
 }
 
 impl Cli {
+    pub fn skills(&self) -> Vec<String> {
+        let mut seen = HashSet::new();
+        let mut out = Vec::with_capacity(self.skill.len());
+        for name in &self.skill {
+            if seen.insert(name.clone()) {
+                out.push(name.clone());
+            }
+        }
+
+        out
+    }
+
     pub fn text(&self) -> Result<Option<String>> {
         let mut stdin_text = String::new();
         if !stdin().is_terminal() {
@@ -323,6 +336,21 @@ mod tests {
         );
     }
 
+    #[test]
+    fn skills_method_dedupes_preserving_first_occurrence() {
+        let cli = parse(&[
+            "--skill", "alpha", "--skill", "beta", "--skill", "alpha", "--skill", "gamma",
+            "--skill", "beta",
+        ]);
+
+        assert_eq!(cli.skills(), vec!["alpha", "beta", "gamma"]);
+    }
+
+    #[test]
+    fn skills_method_returns_empty_when_no_flags() {
+        assert!(parse(&[]).skills().is_empty());
+    }
+
     #[test]
     fn parse_file_flag_single() {
         let cli = parse(&["-f", "file.txt", "question"]);

src/config/app_state.rs

@@ -43,7 +43,7 @@ impl AppState {
         start_mcp_servers: bool,
         abort_signal: AbortSignal,
     ) -> Result<Self> {
-        let vault = Arc::new(Vault::init(&config));
+        let vault = Arc::new(Vault::init(&config)?);
 
         let mcp_registry = McpRegistry::init(
             log_path,

src/config/mod.rs

@@ -490,7 +490,7 @@ impl Config {
             secrets_provider: config.secrets_provider.clone(),
             ..AppConfig::default()
         };
-        let vault = Vault::init(&bootstrap_app);
+        let vault = Vault::init(&bootstrap_app)?;
         let (parsed_config, missing_secrets) = interpolate_secrets(&content, &vault)?;
         if !missing_secrets.is_empty() && !info_flag {
             debug!(

src/graph/llm.rs

@@ -129,13 +129,13 @@ async fn run(
         }
     };
 
-    if policy.skills_enabled
-        && node
-            .tools
-            .as_deref()
-            .map(|t| !t.is_empty())
-            .unwrap_or(false)
-    {
+    let node_has_enabled_skills = node
+        .enabled_skills
+        .as_deref()
+        .map(|s| !s.is_empty())
+        .unwrap_or(false);
+
+    if policy.skills_enabled && node_has_enabled_skills {
         let mut tools = role.enabled_tools().map(|v| v.to_vec()).unwrap_or_default();
         for decl in skill_function_declarations() {
             if !tools.contains(&decl.name) {

src/graph/validator.rs

@@ -228,6 +228,12 @@ impl GraphValidator {
                     ));
                     continue;
                 }
+                if let Err(e) = paths::validate_skill_name(name) {
+                    result.error(ValidationError::new(format!(
+                        "graph 'enabled_skills' contains an invalid skill name: '{name}': {e}"
+                    )));
+                    continue;
+                }
                 if let Some(reason) = check_visibility(name) {
                     result.error(ValidationError::new(format!(
                         "graph 'enabled_skills': {reason}"
@@ -252,6 +258,12 @@ impl GraphValidator {
                     ));
                     continue;
                 }
+                if let Err(e) = paths::validate_skill_name(name) {
+                    result.error(ValidationError::new(format!(
+                        "llm node 'enabled_skills' contains an invalid skill name: '{name}': {e}"
+                    )));
+                    continue;
+                }
                 if let Some(reason) = check_visibility(name) {
                     result.error(ValidationError::with_node(
                         node_id,

src/main.rs

@@ -113,7 +113,7 @@ async fn main() -> Result<()> {
     if vault_flags {
         let cfg = Config::load_with_interpolation(true).await?;
         let app_config = AppConfig::from_config(cfg)?;
-        let vault = Vault::init(&app_config);
+        let vault = Vault::init(&app_config)?;
         return Vault::handle_vault_flags(cli, &vault);
     }
 
@@ -197,17 +197,17 @@ async fn run(
         println!("{skills}");
         return Ok(());
     }
-    if cli.skill.len() == 1 {
-        let name = &cli.skill[0];
+    let skills = cli.skills();
+    if skills.len() == 1 {
+        let name = &skills[0];
         paths::validate_skill_name(name)?;
         if !paths::has_skill(name) {
             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 {
+    } else if skills.len() > 1 {
+        for name in &skills {
             paths::validate_skill_name(name)?;
             if !paths::has_skill(name) {
                 bail!("Skill '{name}' is not installed");
@@ -327,7 +327,7 @@ async fn run(
             .await?;
     }
 
-    for name in &cli.skill {
+    for name in &cli.skills() {
         ctx.load_skill_repl(name, abort_signal.clone()).await?;
     }
 

src/vault/mod.rs

@@ -80,7 +80,7 @@ impl Vault {
         }
     }
 
-    pub fn init(config: &AppConfig) -> Self {
+    pub fn init(config: &AppConfig) -> Result<Self> {
         let mut provider = match &config.secrets_provider {
             Some(p) => p.clone(),
             None => SupportedProvider::Local {
@@ -92,11 +92,10 @@ impl Vault {
         };
 
         if let SupportedProvider::Local { provider_def } = &mut provider {
-            ensure_password_file_initialized(provider_def)
-                .expect("Failed to initialize password file");
+            ensure_password_file_initialized(provider_def)?;
         }
 
-        Self { provider }
+        Ok(Self { provider })
     }
 
     pub fn local_password_file(&self) -> Result<PathBuf> {

src/vault/utils.rs

@@ -355,6 +355,19 @@ fn required_cli_preflight(label: &str, cli: &str, install_url: &str) {
 }
 
 pub fn interpolate_secrets(content: &str, vault: &Vault) -> Result<(String, Vec<String>)> {
+    interpolate_secrets_with(content, vault.auth_hint(), |name| {
+        vault.get_secret(name, false)
+    })
+}
+
+fn interpolate_secrets_with<F>(
+    content: &str,
+    auth_hint: Option<&'static str>,
+    mut get_secret: F,
+) -> Result<(String, Vec<String>)>
+where
+    F: FnMut(&str) -> Result<String>,
+{
     let mut missing_secrets = vec![];
     let mut fatal_error: Option<anyhow::Error> = None;
 
@@ -372,7 +385,7 @@ pub fn interpolate_secrets(content: &str, vault: &Vault) -> Result<(String, Vec<
                     }
 
                     let name = caps[1].trim();
-                    match vault.get_secret(name, false) {
+                    match get_secret(name) {
                         Ok(s) => s,
                         Err(e) => match e.downcast_ref::<SecretError>() {
                             Some(SecretError::NotFound { .. }) => {
@@ -382,7 +395,7 @@ pub fn interpolate_secrets(content: &str, vault: &Vault) -> Result<(String, Vec<
                             Some(SecretError::AuthFailed { .. }) => {
                                 let base =
                                     format!("Failed to fetch secret '{name}' from vault: {e}");
-                                let msg = match vault.auth_hint() {
+                                let msg = match auth_hint {
                                     Some(hint) => format!("{base}\n\nHint: {hint}"),
                                     None => base,
                                 };
@@ -425,3 +438,162 @@ fn set_password_file_permissions(path: &Path) -> Result<()> {
 fn set_password_file_permissions(_path: &Path) -> Result<()> {
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use anyhow::Error;
+    use std::cell::RefCell;
+
+    fn not_found(name: &str) -> Error {
+        Error::new(SecretError::NotFound {
+            key: name.to_string(),
+            provider: "test",
+        })
+    }
+
+    fn auth_failed() -> Error {
+        Error::new(SecretError::AuthFailed {
+            provider: "test",
+            source: anyhow!("auth failure"),
+        })
+    }
+
+    struct Calls(RefCell<Vec<String>>);
+
+    impl Calls {
+        fn new() -> Self {
+            Self(RefCell::new(Vec::new()))
+        }
+
+        fn record(&self, name: &str) {
+            self.0.borrow_mut().push(name.to_string());
+        }
+
+        fn snapshot(&self) -> Vec<String> {
+            self.0.borrow().clone()
+        }
+    }
+
+    #[test]
+    fn interpolates_single_secret_per_line() {
+        let (out, missing) =
+            interpolate_secrets_with("api_key={{API_KEY}}", None, |name| match name {
+                "API_KEY" => Ok("sk-12345".to_string()),
+                other => panic!("unexpected lookup: {other}"),
+            })
+            .unwrap();
+
+        assert_eq!(out, "api_key=sk-12345");
+        assert!(missing.is_empty());
+    }
+
+    #[test]
+    fn regex_matches_each_secret_independently_when_one_per_line() {
+        let calls = Calls::new();
+        let (out, missing) = interpolate_secrets_with("{{ONE}}\nmiddle\n{{TWO}}", None, |name| {
+            calls.record(name);
+            Ok(name.to_lowercase())
+        })
+        .unwrap();
+
+        assert_eq!(calls.snapshot(), vec!["ONE".to_string(), "TWO".to_string()]);
+        assert_eq!(out, "one\nmiddle\ntwo");
+        assert!(missing.is_empty());
+    }
+
+    #[test]
+    fn skips_comment_lines() {
+        let calls = Calls::new();
+
+        let (out, missing) =
+            interpolate_secrets_with("# api_key={{NEVER_FETCHED}}\nreal={{S}}", None, |name| {
+                calls.record(name);
+                Ok("v".to_string())
+            })
+            .unwrap();
+
+        assert_eq!(out, "# api_key={{NEVER_FETCHED}}\nreal=v");
+        assert!(missing.is_empty());
+        assert_eq!(calls.snapshot(), vec!["S".to_string()]);
+    }
+
+    #[test]
+    fn missing_secrets_become_empty_strings_and_are_reported() {
+        let (out, missing) = interpolate_secrets_with(
+            "a={{HAVE}}\nb={{MISSING_1}}\nc={{MISSING_2}}",
+            None,
+            |name| match name {
+                "HAVE" => Ok("present".to_string()),
+                missing => Err(not_found(missing)),
+            },
+        )
+        .unwrap();
+
+        assert_eq!(out, "a=present\nb=\nc=");
+        assert_eq!(
+            missing,
+            vec!["MISSING_1".to_string(), "MISSING_2".to_string()]
+        );
+    }
+
+    #[test]
+    fn fatal_failure_short_circuits_remaining_lines() {
+        let calls = Calls::new();
+        let result =
+            interpolate_secrets_with("a={{S1}}\nb={{S2}}\nc={{S3}}\nd={{S4}}", None, |name| {
+                calls.record(name);
+                match name {
+                    "S1" => Ok("first".to_string()),
+                    "S2" => Err(auth_failed()),
+                    other => Ok(format!("late-{other}")),
+                }
+            });
+
+        let err = result.unwrap_err().to_string();
+        assert!(
+            err.contains("S2"),
+            "error should name the offending secret, got: {err}"
+        );
+        assert_eq!(
+            calls.snapshot(),
+            vec!["S1".to_string(), "S2".to_string()],
+            "lookups must stop at the failing secret - S3 and S4 should never be fetched"
+        );
+    }
+
+    #[test]
+    fn auth_failure_appends_hint_when_provided() {
+        let result = interpolate_secrets_with(
+            "k={{K}}",
+            Some("run `coyote --authenticate` to reauth"),
+            |_| Err(auth_failed()),
+        );
+
+        let err = result.unwrap_err().to_string();
+
+        assert!(err.contains("Hint:"), "expected hint in error, got: {err}");
+        assert!(
+            err.contains("coyote --authenticate"),
+            "expected hint contents, got: {err}"
+        );
+    }
+
+    #[test]
+    fn regex_greedy_capture_collapses_multi_secret_line_into_single_lookup() {
+        let calls = Calls::new();
+        let (out, missing) = interpolate_secrets_with("url={{URL}} key={{KEY}}", None, |name| {
+            calls.record(name);
+            Err(not_found(name))
+        })
+        .unwrap();
+
+        assert_eq!(
+            calls.snapshot(),
+            vec!["URL}} key={{KEY".to_string()],
+            "greedy regex spans first {{ to last }}, collapsing the whole line into one bogus lookup"
+        );
+        assert_eq!(out, "url=");
+        assert_eq!(missing, vec!["URL}} key={{KEY".to_string()]);
+    }
+}