fix: privilege leak when unloading skills and leaving tool scope untouched

Release v0.4.0: Graph-based agents, remote asset installation, self-update and god-config refactor

assets/agents/coder/scripts/route_review_result.sh

@@ -14,21 +14,6 @@ review_attempts=$(echo "$state" | jq -r '.review_attempts // 0')
 max_review_attempts=$(echo "$state" | jq -r '.max_review_attempts // 1')
 review_notes=$(echo "$state" | jq -r '.review_notes // ""')
 
-if [[ "$review_clean" != "true" && "$review_clean" != "false" ]]; then
-  echo "ERROR: review_clean must be boolean ('true'/'false'); got: $review_clean" >&2
-  exit 1
-fi
-
-if ! [[ "$review_attempts" =~ ^[0-9]+$ ]]; then
-  echo "ERROR: review_attempts must be a non-negative integer; got: $review_attempts" >&2
-  exit 1
-fi
-
-if ! [[ "$max_review_attempts" =~ ^[0-9]+$ ]]; then
-  echo "ERROR: max_review_attempts must be a non-negative integer; got: $max_review_attempts" >&2
-  exit 1
-fi
-
 if [[ "$review_clean" == "true" ]]; then
   jq -nc '{"_next": "end_success"}'
   exit 0

assets/agents/explore/config.yaml

@@ -1,10 +1,9 @@
 name: explore
 description: Fast codebase exploration agent - finds patterns, structures, and relevant files. Designed to be fanned out 2-5 in parallel by orchestrators.
-version: 3.0.0
+version: 2.0.0
 
 skills_enabled: true
-enabled_skills:
+enabled_skills: []
-  - ai-slop-remover
 
 variables:
   - name: project_dir
@@ -23,85 +22,64 @@ global_tools:
 instructions: |
   You are a codebase explorer. Your job: Search, find, report. Nothing else.
 
-  ## Step 0: Load your skills
-
-  At the start of every exploration, call `skill__load` for `ai-slop-remover`. Your findings go directly into the orchestrator's synthesis, so concise, slop-free output is the contract. Apply the skill's standards to your final findings block:
-
-  - No filler ("It's important to note that…", "Let me explain…"). Just the finding.
-  - No flattery, no padding, no status updates about your process.
-  - No multi-paragraph commentary — bullet points with code snippets are enough.
-
   ## You may be one of many parallel explorers
 
   Orchestrators (like Sisyphus) often fan out 2-5 explore agents at once, each covering a different angle of the same question. Assume you are ONE narrow slice of a larger investigation. Stay strictly within YOUR slice as defined by the prompt — don't broaden scope to cover what other parallel explorers might be handling.
 
   If the prompt says "find auth middleware", you find auth middleware. You do NOT also tour the routing layer, the error system, and the database connection pool. Narrow scope is the contract.
 
-  ## Investigation methodology
+  ## Your mission
 
-  Before searching, build a quick mental model. Then narrow in. Then read.
+  1. Search for relevant files and patterns within YOUR slice.
+  2. Read key files to understand structure.
+  3. Report findings concisely.
+  4. Signal completion with `EXPLORE_COMPLETE`.
 
-  1. **Frame the question.** What kind of artifact am I looking for? Symbols (struct/class/function)? File patterns? Configuration? Implementation details? Tests? Different artifact kinds use different tools.
+  ## File reading strategy (minimize token usage)
 
-  2. **Find first, read second.** Never `fs_read` a file without knowing why you're reading it.
+  1. **Find first, read second** — never read a file without knowing why.
-
+  2. **Use grep to locate** — `fs_grep --pattern "struct User" --include "*.rs"` finds where things are.
-  3. **Build a directory mental model with `fs_ls` and `fs_glob`** — `fs_ls src/` to see what's there; `fs_glob '**/*.rs' src/` to see which files exist by name.
+  3. **Use glob to discover** — `fs_glob --pattern "*.rs" --path src/` finds files by name.
-
+  4. **Prefer `fs_read` with offset/limit** — `fs_read --path "src/main.rs" --offset 50 --limit 30` reads lines 50-79 only. `fs_read` adds line numbers but TRUNCATES long lines (over 2000 chars) and caps output at 2000 lines by default.
-  4. **Locate symbols with `fs_grep`** — for finding where things live across the codebase. `fs_grep --pattern "fn handle_request" --include "*.rs"` is faster than reading files.
+  5. **Use `fs_cat` only when you need the entire file untruncated** — for exploration this should be rare. If you find yourself reaching for `fs_cat`, ask whether `fs_grep` + a targeted `fs_read` would answer your question instead.
-
+  6. **Never read entire large files** — if a file is 500+ lines, read the relevant section only.
-  5. **Read targeted sections with `fs_read --offset/--limit`** — `fs_read --path "src/main.rs" --offset 50 --limit 30` reads lines 50-79 only. `fs_read` adds line numbers but TRUNCATES long lines (over 2000 chars) and caps output at 2000 lines by default.
-
-  6. **Use `fs_cat` only when you need the full untruncated file** — rare in exploration. If you reach for `fs_cat`, ask whether `fs_grep` + targeted `fs_read` would answer your question with less context spend.
-
-  7. **Never read entire large files** — for files 500+ lines, read the relevant section only.
 
   ## Available actions
 
-  - `fs_grep --pattern "struct User" --include "*.rs"` — find content across files in a directory tree
+  - `fs_grep --pattern "struct User" --include "*.rs"` — find content across files
-  - `fs_grep --pattern "TODO" --path "src/main.rs"` — find content within a single file (--include is ignored in this mode)
   - `fs_glob --pattern "*.rs" --path src/` — find files by name pattern
   - `fs_read --path "src/main.rs"` — read a TRUNCATED view with line numbers (default 2000 lines, lines over 2000 chars cut off)
-  - `fs_read --path "src/main.rs" --offset 100 --limit 50` — read lines 100-149 only (line numbers; truncation rules still apply)
+  - `fs_read --path "src/main.rs" --offset 100 --limit 50` — read lines 100-149 only (with line numbers, truncation rules still apply)
   - `fs_cat --path "src/main.rs"` — read the FULL untruncated file (no line numbers); use only when you actually need every line
   - `fs_ls --path "src/"` — list directory contents
 
-  ## When to use the web (ddg-search MCP)
-
-  Rarely. You are a CODEBASE explorer, not a web researcher. Use the web only when the codebase references an external library/framework whose documented behavior is the answer to the question (e.g., "how does Tokio's #[tokio::main] expand"), and the answer isn't in the local code. For internal questions ("how does OUR auth work"), grep the codebase — never the web.
-
   ## Output format
 
-  Always end your response with a structured findings block. Sisyphus reads this verbatim and may paste sections directly into delegation prompts for a coder agent, so the structure matters:
+  Always end your response with a findings summary. Include actual code snippets when they show the pattern — file paths alone are not enough for the orchestrator to delegate downstream:
 
   ```
   FINDINGS:
-  - [One-line concrete fact about what you found]
+  - [Key finding 1]
-  - [Another one-line fact]
+  - [Key finding 2]
-  - Relevant files: [list of paths, no commentary]
+  - Relevant files: [list]
 
   Code patterns (paste actual lines):
   - From `path/to/file.ext` lines N-M:
-    <5-20 lines of actual code that show the pattern>
+    <snippet>
-  - From `path/to/other.ext` lines N-M:
-    <another snippet>
-
-  Open questions (only if any):
-  - [Anything you couldn't determine and the orchestrator should clarify or delegate elsewhere]
 
   EXPLORE_COMPLETE
   ```
 
-  Pasting actual code lines (5-20 per pattern) lets the orchestrator hand snippets directly to a coder agent without re-exploration. That is the entire point of your existence in a parallel research phase. File paths alone make downstream delegation impossible — the coder would have to re-do your work.
+  Pasting actual code lines (5-20 lines per pattern) lets the orchestrator hand the snippet directly to a coder agent without re-exploration. That is the whole point of your existence in a fanned-out research phase.
 
   ## Rules
 
-  1. **Be fast.** Don't read every file, read representative ones.
+  1. **Be fast** — don't read every file, read representative ones.
-  2. **Stay in your slice.** Narrow scope is the contract.
+  2. **Stay in your slice** — narrow scope is the contract.
-  3. **Be concise.** Report findings, not your process. Apply the `ai-slop-remover` skill to your output.
+  3. **Be concise** — report findings, not your process.
-  4. **Never modify files.** You are read-only.
+  4. **Never modify files** — you are read-only.
-  5. **Limit reads.** Target around 5 file reads per exploration; go higher only when the question genuinely requires it.
+  5. **Limit reads** — max 5 file reads per exploration.
-  6. **Paste code snippets.** File paths alone make downstream delegation impossible.
+  6. **Paste code snippets** — file paths alone make downstream delegation impossible.
-  7. **Report what you didn't find.** If the prompt asked for X and X doesn't exist in your slice, say so explicitly — don't pad your findings with adjacent material to hide the gap.
 
   ## Context
   - Project: {{project_dir}}

assets/agents/sisyphus/config.yaml

@@ -239,45 +239,6 @@ instructions: |
 
   **No evidence = not complete.** Mark a todo `completed` only after evidence is collected.
 
-  ### Independent code review (post-coder, non-trivial work)
-
-  After completing delegated `coder` work, spawn `code-reviewer` for an independent review pass if ANY of these are true:
-
-  1. **2+ coder agents were spawned** for this task (multi-component change; no single coder saw the whole picture)
-  2. **A single coder touched 5+ files** (broad-scope change; harder for self-review to hold in one context)
-  3. **The change crosses architectural boundaries** — auth, public APIs, security-sensitive paths, schema/migration files, configuration that affects multiple services
-  4. **You judge the change as architecturally significant** even if 1-3 don't trigger
-
-  If none of these fire, the work is "single coder, narrow scope, mechanical" — coder's internal `self_review` is sufficient.
-
-  **Why this matters.** Coder's `self_review` is a same-agent check: the agent that wrote the code reviews its own diff. It catches surface slop and obvious mistakes, but it's structurally weak at catching cross-cutting issues across parallel coders, subtle design problems the author justified to themselves, and rationalized "not my job" footguns. `code-reviewer` is independent — no commitment to the prior design decisions. The independence is the value, and it's how real-world engineering catches what authors miss.
-
-  **Spawn pattern:**
-
-  ```
-  agent__spawn --agent code-reviewer --prompt "Review the changes from the recent coder run(s) for this task.
-
-  Original request: <one-line summary of what the user asked for>
-  Scope: <which directories or files the changes are expected to touch>
-
-  Coder summaries:
-  - <coder 1 session_id>: <plan_summary from CODER_COMPLETE>
-  - <coder 2 session_id>: <plan_summary if multiple coders ran>
-
-  Run `get_diff` against the staged or recent changes, fan out file-reviewers per changed file as usual, and synthesize."
-  ```
-
-  ### Handling code-reviewer findings
-
-  - **🔴 CRITICAL** findings block completion. Spawn `coder` to fix — preferably the SAME session as the original coder (`agent__spawn --session_id <id> --prompt "Fix: <critical findings pasted verbatim>"`). Do NOT re-spawn `code-reviewer` automatically after the fix; coder's own `self_review` on the fix is sufficient unless the fix itself was substantial (5+ files or architectural).
-  - **🟡 WARNING** findings are blocking unless the work was explicitly scoped to defer them. If unsure, ASK the user via `user__ask` whether to fix or accept.
-  - **🟢 SUGGESTION / 💡 NITPICK** findings are informational. Surface them to the user with the final report. Do not block on them.
-  - **`Pre-existing, out of scope:` findings** — surface to the user but do not act on them. They predate this work and aren't the current task's responsibility.
-
-  ### When NOT to re-spawn code-reviewer
-
-  After a fix-loop completes, do not automatically re-run `code-reviewer` unless the fix itself triggers the same thresholds (2+ coders, 5+ files, architectural). Each `code-reviewer` invocation fans out N file-reviewers per changed file; spurious re-runs burn budget without proportional value. Trust coder's `self_review` on bounded fixes.
-
   ## File Operations (Direct Edits)
 
   When you write or modify files yourself (rather than delegating to coder):

assets/functions/tools/fs_grep.sh

@@ -28,8 +28,6 @@ main() {
     local grep_args=(-nH --color=never)
 
     if [[ -d "$search_path" ]]; then
-        # Use -r (not -R) so symlinks to directories are NOT followed - this avoids
-        # infinite loops on pathological symlink cycles (e.g. `ln -s . loop`).
         grep_args+=(-r)
         grep_args+=(
             --exclude-dir='.git'

models.yaml

@@ -3,62 +3,6 @@
 #  - https://platform.openai.com/docs/api-reference/chat
 - provider: openai
   models:
-    - name: gpt-5.5
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 5
-      output_price: 30
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.5-pro
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 30
-      output_price: 180
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.4
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 2.5
-      output_price: 15
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.4-pro
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 30
-      output_price: 180
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.4-mini
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 0.75
-      output_price: 4.5
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.4-nano
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 0.2
-      output_price: 1.25
-      supports_vision: true
-      supports_function_calling: true
-    - name: gpt-5.3-codex
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 1.75
-      output_price: 14
-      supports_vision: true
-      supports_function_calling: true
-    - name: chat-latest
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 5
-      output_price: 30
-      supports_vision: true
-      supports_function_calling: true
     - name: gpt-5.2
       max_input_tokens: 400000
       max_output_tokens: 128000
@@ -1596,55 +1540,6 @@
 #  - https://openrouter.ai/docs/api-reference/chat-completion
 - provider: openrouter
   models:
-    - name: openai/gpt-5.5
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 5
-      output_price: 30
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.5-pro
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 30
-      output_price: 180
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.4
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 2.5
-      output_price: 15
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.4-pro
-      max_input_tokens: 1050000
-      max_output_tokens: 128000
-      input_price: 30
-      output_price: 180
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.4-mini
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 0.75
-      output_price: 4.5
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.4-nano
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 0.2
-      output_price: 1.25
-      supports_vision: true
-      supports_function_calling: true
-    - name: openai/gpt-5.3-codex
-      max_input_tokens: 400000
-      max_output_tokens: 128000
-      input_price: 1.75
-      output_price: 14
-      supports_vision: true
-      supports_function_calling: true
     - name: openai/gpt-5.2
       max_input_tokens: 400000
       max_output_tokens: 128000

src/cli/completer.rs

@@ -137,16 +137,13 @@ 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) => match Vault::init(&app_config) {
+        Ok(app_config) => Vault::init(&app_config)
-            Ok(vault) => vault
+            .list_secrets(false)
-                .list_secrets(false)
+            .unwrap_or_default()
-                .unwrap_or_default()
+            .into_iter()
-                .into_iter()
+            .filter(|s| s.starts_with(&*cur))
-                .filter(|s| s.starts_with(&*cur))
+            .map(CompletionCandidate::new)
-                .map(CompletionCandidate::new)
+            .collect(),
-                .collect(),
-            Err(_) => vec![],
-        },
         Err(_) => vec![],
     }
 }

src/cli/mod.rs

@@ -10,7 +10,6 @@ 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)]
@@ -164,18 +163,6 @@ 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() {
@@ -336,21 +323,6 @@ 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/client/common.rs

@@ -354,9 +354,7 @@ pub async fn create_config(
         "type": client,
     });
     for (key, desc, help_message, is_secret) in prompts {
-        let env_name = format!("{client}-{key}")
+        let env_name = format!("{client}_{key}").to_ascii_uppercase();
-            .to_ascii_uppercase()
-            .replace("_", "-");
         let required = std::env::var(&env_name).is_err();
         let value = if !is_secret {
             prompt_input_string(desc, required, *help_message)?

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, bail};
+use anyhow::{Context, Result, anyhow};
 use gman::providers::SupportedProvider;
 use indexmap::IndexMap;
 use serde::Deserialize;
@@ -216,7 +216,6 @@ 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)?;
         }
@@ -226,28 +225,11 @@ 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() {
-                bail!("No available model");
+                anyhow::bail!("No available model");
             }
             self.model_id = models[0].id();
         }

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/install_remote.rs

@@ -1,3 +1,10 @@
+use anyhow::{Context, Result, bail};
+use indexmap::IndexMap;
+use inquire::{Confirm, Select};
+use std::ffi::{OsStr, OsString};
+use std::fs;
+use std::path::{Path, PathBuf};
+
 use crate::config::{InstallFilter, paths};
 #[cfg(not(windows))]
 use crate::function::Language;
@@ -5,13 +12,6 @@ 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, anyhow, bail};
-use indexmap::IndexMap;
-use indoc::formatdoc;
-use inquire::{Confirm, Select};
-use std::ffi::{OsStr, OsString};
-use std::fs;
-use std::path::{Path, PathBuf};
 
 pub fn install_remote(git_url: &str, filter: Option<InstallFilter>, force: bool) -> Result<()> {
     let (url, reference) = parse_url_with_ref(git_url)?;
@@ -418,26 +418,6 @@ 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 {
@@ -751,21 +731,8 @@ fn merge_mcp_json(
         serde_json::to_string_pretty(&merged).context("failed to serialize merged mcp.json")?;
     write_atomically(&final_path, &serialized)?;
 
-    let vault = Vault::init_bare()?;
+    let vault = Vault::init_bare();
-    let missing = match interpolate_secrets(&serialized, &vault) {
+    let (_parsed, missing) = interpolate_secrets(&serialized, &vault)?;
-        Ok((_, missing)) => missing,
-        Err(e) => {
-            eprintln!(
-                "{}",
-                formatdoc! {"
-                Skipping secret resolution for merged mcp.json: {e:#}
-                Continuing without resolving missing secrets
-                You may need to add any additional missing secrets to the vault manually.
-            "}
-            );
-            Vec::new()
-        }
-    };
     let mut deduped: Vec<String> = Vec::new();
     for s in missing {
         if !deduped.contains(&s) {
@@ -893,7 +860,7 @@ fn handle_missing_secrets(missing: &[String]) -> Result<()> {
 }
 
 fn prompt_for_each_secret(missing: &[String]) -> Result<(Vec<String>, Vec<String>)> {
-    let mut vault = Vault::init_bare()?;
+    let mut vault = Vault::init_bare();
     let mut password_file_ensured = false;
     let mut added = Vec::new();
     let mut deferred = Vec::new();
@@ -947,62 +914,6 @@ fn print_secret_summary(added: &[String], deferred: &[String]) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::utils::get_env_name;
-    use serial_test::serial;
-    use std::env;
-    use std::time::{SystemTime, UNIX_EPOCH};
-
-    struct TestVaultConfigGuard {
-        dir_key: String,
-        file_key: String,
-        previous_dir: Option<OsString>,
-        previous_file: Option<OsString>,
-        path: PathBuf,
-    }
-
-    impl TestVaultConfigGuard {
-        fn new(label: &str) -> Self {
-            let dir_key = get_env_name("config_dir");
-            let file_key = get_env_name("config_file");
-            let previous_dir = env::var_os(&dir_key);
-            let previous_file = env::var_os(&file_key);
-            let unique = SystemTime::now()
-                .duration_since(UNIX_EPOCH)
-                .unwrap()
-                .as_nanos();
-            let path = env::temp_dir().join(format!("coyote-vault-test-{label}-{unique}"));
-            fs::create_dir_all(&path).unwrap();
-            let config_path = path.join("config.yaml");
-            fs::write(&config_path, "{}").unwrap();
-            unsafe {
-                env::set_var(&dir_key, &path);
-                env::set_var(&file_key, &config_path);
-            }
-            Self {
-                dir_key,
-                file_key,
-                previous_dir,
-                previous_file,
-                path,
-            }
-        }
-    }
-
-    impl Drop for TestVaultConfigGuard {
-        fn drop(&mut self) {
-            unsafe {
-                match &self.previous_dir {
-                    Some(p) => env::set_var(&self.dir_key, p),
-                    None => env::remove_var(&self.dir_key),
-                }
-                match &self.previous_file {
-                    Some(p) => env::set_var(&self.file_key, p),
-                    None => env::remove_var(&self.file_key),
-                }
-            }
-            let _ = fs::remove_dir_all(&self.path);
-        }
-    }
 
     #[test]
     fn parse_url_no_ref() {
@@ -1342,9 +1253,7 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn merge_into_empty_local_adds_all_remote_servers() {
-        let _guard = TestVaultConfigGuard::new("merge-empty");
         let dir = fresh_temp_dir("merge-empty-");
         let remote = dir.join("remote.json");
         let target = dir.join("target.json");
@@ -1361,9 +1270,7 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn merge_force_replaces_local_on_conflict() {
-        let _guard = TestVaultConfigGuard::new("merge-force");
         let dir = fresh_temp_dir("merge-force-");
         let remote = dir.join("remote.json");
         let target = dir.join("target.json");
@@ -1429,9 +1336,7 @@ mod tests {
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
-    #[serial]
     async fn merge_detects_missing_secrets_in_output() {
-        let _guard = TestVaultConfigGuard::new("merge-secret");
         let dir = fresh_temp_dir("merge-secret-");
         let remote = dir.join("remote.json");
         let target = dir.join("target.json");
@@ -1447,9 +1352,7 @@ mod tests {
     }
 
     #[test]
-    #[serial]
     fn merge_is_idempotent_on_re_run() {
-        let _guard = TestVaultConfigGuard::new("merge-idempotent");
         let dir = fresh_temp_dir("merge-idempotent-");
         let remote = dir.join("remote.json");
         let target = dir.join("target.json");

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!(
@@ -685,7 +685,7 @@ pub async fn create_config_file(config_path: &Path) -> Result<()> {
 
     let provider_choice = prompt_provider_choice()?;
     let mut vault = match &provider_choice {
-        None => Vault::default_local(),
+        None => Vault::init_bare(),
         Some(provider) => Vault {
             provider: provider.clone(),
         },

src/config/paths.rs

@@ -270,7 +270,6 @@ pub fn list_skills() -> Vec<String> {
                 && file_type.is_dir()
                 && let Some(name) = entry.file_name().to_str()
                 && entry.path().join("SKILL.md").is_file()
-                && validate_skill_name(name).is_ok()
             {
                 names.push(name.to_string());
             }
@@ -282,6 +281,10 @@ 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()
 }
 
@@ -304,7 +307,6 @@ pub fn local_models_override() -> Result<Vec<ProviderModels>> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::{fs, time};
 
     #[test]
     fn validate_skill_name_accepts_alphanumerics_and_dashes() {
@@ -341,43 +343,12 @@ mod tests {
     }
 
     #[test]
-    fn has_skill_returns_false_for_missing_paths() {
+    fn has_skill_returns_false_for_invalid_names() {
-        for absent in ["definitely-not-installed-skill-xyz", "another-missing"] {
+        for bad in ["", "../escape", "foo/bar", ".hidden", "with space"] {
             assert!(
-                !has_skill(absent),
+                !has_skill(bad),
-                "has_skill({absent:?}) should be false for a missing skill"
+                "has_skill({bad:?}) should be false for an invalid name"
             );
         }
     }
-
-    #[test]
-    fn list_skills_skips_invalid_directory_names() {
-        let unique = time::SystemTime::now()
-            .duration_since(time::UNIX_EPOCH)
-            .unwrap()
-            .as_nanos();
-        let root = env::temp_dir().join(format!("coyote-list-skills-test-{unique}"));
-        fs::create_dir_all(&root).unwrap();
-        let prev = env::var_os(get_env_name("skills_dir"));
-        unsafe {
-            env::set_var(get_env_name("skills_dir"), &root);
-        }
-
-        for name in ["valid-skill", "with space", ".hidden", "dot.name"] {
-            let dir = root.join(name);
-            fs::create_dir_all(&dir).unwrap();
-            fs::write(dir.join("SKILL.md"), "body").unwrap();
-        }
-
-        let listed = list_skills();
-        assert_eq!(listed, vec!["valid-skill".to_string()]);
-
-        unsafe {
-            match prev {
-                Some(v) => env::set_var(get_env_name("skills_dir"), v),
-                None => env::remove_var(get_env_name("skills_dir")),
-            }
-        }
-        let _ = fs::remove_dir_all(&root);
-    }
 }

src/config/request_context.rs

@@ -1229,12 +1229,7 @@ impl RequestContext {
                     .collect();
 
                 if let Some(ref tool_names) = role_filter {
-                    agent_functions.retain(|v| {
+                    agent_functions.retain(|v| tool_names.contains(&v.name));
-                        tool_names.contains(&v.name)
-                            || (!matches!(agent.skills_enabled(), Some(false))
-                                && v.name.starts_with(SKILL_FUNCTION_PREFIX))
-                            || v.name.starts_with(USER_FUNCTION_PREFIX)
-                    });
                 }
 
                 let tool_names: HashSet<String> = agent_functions
@@ -1738,26 +1733,6 @@ impl RequestContext {
             "enabled_skills" => {
                 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 = self.app.config.visible_skills.as_deref();
-                    for name in names {
-                        paths::validate_skill_name(name)?;
-                        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");
-                                }
-                            }
-                        }
-                    }
-                }
                 self.update_app_config(|app| app.enabled_skills = parsed.clone());
             }
             "skills_enabled" => {
@@ -2671,9 +2646,7 @@ impl RequestContext {
 
         self.skill_registry.insert(skill)?;
         if let Err(e) = self.refresh_tool_scope(abort_signal).await {
-            if let Err(unload_err) = self.skill_registry.unload(name) {
+            let _ = self.skill_registry.unload(name);
-                warn!("Failed to unload skill '{name}' during error recovery: {unload_err}");
-            }
             bail!("Loaded skill '{name}' but failed to refresh tool scope: {e}");
         }
 
@@ -2682,15 +2655,10 @@ impl RequestContext {
     }
 
     pub async fn unload_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> {
-        let skill = self.skill_registry.unload(name)?;
+        self.skill_registry.unload(name)?;
 
         if let Err(e) = self.refresh_tool_scope(abort_signal).await {
-            if let Err(restore_err) = self.skill_registry.insert(skill) {
+            eprintln!("Warning: unloaded skill '{name}' but tool scope refresh failed: {e}");
-                warn!(
-                    "Failed to restore skill '{name}' after tool-scope refresh failure: {restore_err}"
-                );
-            }
-            bail!("Unloaded skill '{name}' but failed to refresh tool scope; restored: {e}");
         }
 
         println!("✓ Unloaded skill '{name}'.");

src/config/skill_policy.rs

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

src/function/skill.rs

@@ -104,13 +104,8 @@ pub async fn handle_skill_tool(
 fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result<Value> {
     let mcp_on = ctx.app.config.mcp_server_support;
 
-    let visible_names: Vec<String> = match ctx.app.config.visible_skills.as_deref() {
-        Some(list) => list.to_vec(),
-        None => paths::list_skills(),
-    };
-
     let mut entries = Vec::new();
-    for name in visible_names {
+    for name in paths::list_skills() {
         if !policy.allows(&name) {
             continue;
         }
@@ -198,10 +193,7 @@ async fn handle_load(
     }
 
     if let Err(e) = ctx.refresh_tool_scope(create_abort_signal()).await {
-        if let Err(unload_err) = ctx.skill_registry.unload(name) {
+        let _ = ctx.skill_registry.unload(name);
-            warn!("Failed to unload skill '{name}' during error recovery: {unload_err}");
-        }
-
         return Ok(json!({
             "error": format!("Loaded skill '{name}' but failed to refresh tool scope: {e}")
         }));
@@ -220,20 +212,13 @@ async fn handle_unload(ctx: &mut RequestContext, args: &Value) -> Result<Value>
         _ => return Ok(json!({"error": "name is required"})),
     };
 
-    if let Err(e) = paths::validate_skill_name(name) {
-        return Ok(json!({"error": e.to_string()}));
-    }
-
     let skill = match ctx.skill_registry.unload(name) {
         Ok(s) => s,
         Err(e) => return Ok(json!({"error": e.to_string()})),
     };
 
     if let Err(e) = ctx.refresh_tool_scope(create_abort_signal()).await {
-        if let Err(insert_err) = ctx.skill_registry.insert(skill) {
+        let _ = ctx.skill_registry.insert(skill);
-            warn!("Failed to restore skill '{name}' after unload recovery: {insert_err}");
-        }
-
         return Ok(json!({
             "error": format!(
                 "Unloaded skill '{name}' but failed to refresh tool scope; restored: {e}"

src/graph/llm.rs

@@ -3,7 +3,6 @@ use super::structured;
 use super::types::LlmNode;
 use crate::client::{Model, ModelType, call_chat_completions};
 use crate::config::{Input, RequestContext, Role, RoleLike, SkillPolicy};
-use crate::function::skill::skill_function_declarations;
 use crate::utils::create_abort_signal;
 use anyhow::{Context, Error, Result, anyhow, bail};
 use serde_json::Value;
@@ -106,7 +105,7 @@ async fn run(
     let (regular_tools, mcp_servers) = categorize_tools(node.tools.as_deref());
     validate_tools_subset(&regular_tools, &mcp_servers, parent_ctx)?;
 
-    let mut role = build_inline_role(
+    let role = build_inline_role(
         node,
         instructions.as_deref(),
         &regular_tools,
@@ -116,29 +115,12 @@ async fn run(
 
     let saved_agent_skill_state = swap_in_node_skill_policy(node, parent_ctx);
 
-    let policy = match SkillPolicy::effective(
+    let policy = SkillPolicy::effective(
         &parent_ctx.app.config,
         parent_ctx.role.as_ref(),
         parent_ctx.agent.as_ref(),
         parent_ctx.session.as_ref(),
-    ) {
+    )?;
-        Ok(p) => p,
-        Err(e) => {
-            restore_agent_skill_policy(parent_ctx, saved_agent_skill_state);
-            return Err(e);
-        }
-    };
-
-    if policy.skills_enabled {
-        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) {
-                tools.push(decl.name);
-            }
-        }
-        role.set_enabled_tools(Some(tools));
-    }
-
     let composed_role = parent_ctx.skill_registry.effective_role(&role, &policy);
 
     let saved_role = parent_ctx.role.clone();

src/graph/validator.rs

@@ -93,7 +93,6 @@ impl AgentValidationContext {
 pub struct GraphValidator {
     base_dir: PathBuf,
     agent_ctx: Option<AgentValidationContext>,
-    skill_exists: fn(&str) -> bool,
 }
 
 impl GraphValidator {
@@ -101,7 +100,6 @@ impl GraphValidator {
         Self {
             base_dir: base_dir.into(),
             agent_ctx: None,
-            skill_exists: paths::has_skill,
         }
     }
 
@@ -110,12 +108,6 @@ impl GraphValidator {
         self
     }
 
-    #[cfg(test)]
-    pub fn with_skill_exists(mut self, f: fn(&str) -> bool) -> Self {
-        self.skill_exists = f;
-        self
-    }
-
     pub fn validate(&self, graph: &Graph) -> ValidationResult {
         let mut result = ValidationResult::default();
         self.validate_node_references(graph, &mut result);
@@ -199,49 +191,6 @@ impl GraphValidator {
     }
 
     fn validate_llm_skills(&self, graph: &Graph, result: &mut ValidationResult) {
-        let visible_skills = self
-            .agent_ctx
-            .as_ref()
-            .and_then(|c| c.app_config.visible_skills.as_deref());
-
-        let skill_exists = self.skill_exists;
-        let has_agent_ctx = self.agent_ctx.is_some();
-        let check_visibility = |name: &str| -> Option<String> {
-            if !has_agent_ctx {
-                return None;
-            }
-
-            match visible_skills {
-                Some(list) if !list.iter().any(|s| s == name) => Some(format!(
-                    "'{name}' is not in the global 'visible_skills' allow-list"
-                )),
-                None if !skill_exists(name) => Some(format!("'{name}' is not installed")),
-                _ => None,
-            }
-        };
-
-        if let Some(graph_skills) = &graph.enabled_skills {
-            for name in graph_skills {
-                if name.trim().is_empty() {
-                    result.error(ValidationError::new(
-                        "graph 'enabled_skills' contains an empty skill name",
-                    ));
-                    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}"
-                    )));
-                }
-            }
-        }
-
         for (node_id, node) in &graph.nodes {
             let NodeType::Llm(llm) = &node.node_type else {
                 continue;
@@ -258,22 +207,6 @@ impl GraphValidator {
                     ));
                     continue;
                 }
-                if let Err(e) = paths::validate_skill_name(name) {
-                    result.error(ValidationError::with_node(
-                        node_id,
-                        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,
-                        format!("llm node 'enabled_skills': {reason}"),
-                    ));
-                    continue;
-                }
-
                 if let Some(graph_skills) = &graph.enabled_skills
                     && !graph_skills.iter().any(|g| g == name)
                 {
@@ -1392,7 +1325,7 @@ mod tests {
     }
 
     fn validator() -> GraphValidator {
-        GraphValidator::new(env::current_dir().unwrap()).with_skill_exists(|_: &str| true)
+        GraphValidator::new(env::current_dir().unwrap())
     }
 
     #[test]

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,18 +197,14 @@ async fn run(
         println!("{skills}");
         return Ok(());
     }
-    let skills = cli.skills();
+    if cli.skill.len() == 1 && !paths::has_skill(&cli.skill[0]) {
-    if skills.len() == 1 {
+        let name = &cli.skill[0];
-        let name = &skills[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)?;
+    if cli.skill.len() > 1 {
-            return Ok(());
+        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 +323,7 @@ async fn run(
             .await?;
     }
 
-    for name in &cli.skills() {
+    for name in &cli.skill {
         ctx.load_skill_repl(name, abort_signal.clone()).await?;
     }
 

src/repl/mod.rs

@@ -708,8 +708,6 @@ pub async fn run_repl_command(
                         let name = s.strip_prefix("skill").unwrap_or("").trim();
                         if name.is_empty() {
                             println!("Usage: .edit skill <name>");
-                        } else if let Err(e) = paths::validate_skill_name(name) {
-                            bail!(e);
                         } else if !paths::has_skill(name) {
                             bail!(
                                 "Skill '{name}' is not installed (expected at {})",

src/vault/mod.rs

@@ -1,9 +1,6 @@
 mod utils;
 
-use std::fs::read_to_string;
 use std::path::PathBuf;
-
-use crate::config::paths;
 pub use utils::create_vault_password_file;
 pub use utils::interpolate_secrets;
 pub use utils::prompt_provider_choice;
@@ -17,13 +14,11 @@ use gman::providers::SecretProvider;
 use gman::providers::SupportedProvider;
 use gman::providers::local::LocalProvider;
 use inquire::{Password, PasswordDisplayMode, required};
-use log::warn;
-use serde_yaml::Value;
 use std::sync::{Arc, LazyLock};
 use tokio::runtime::Handle;
 use uuid::Uuid;
 
-pub static SECRET_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{\{([^{}]+)}}").unwrap());
+pub static SECRET_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{\{(.+)}}").unwrap());
 
 #[derive(Debug, Default, Clone)]
 pub struct Vault {
@@ -33,54 +28,22 @@ pub struct Vault {
 pub type GlobalVault = Arc<Vault>;
 
 impl Vault {
-    pub fn init_bare() -> Result<Self> {
+    pub fn init_bare() -> Self {
-        let config_path = paths::config_file();
+        let vault_password_file = AppConfig::default().vault_password_file();
-        if !config_path.exists() {
+        let local_provider = LocalProvider {
-            bail!(
+            password_file: Some(vault_password_file),
-                "Coyote config not found at {}. Run first-run setup before using the vault.",
+            git_branch: None,
-                config_path.display()
+            ..LocalProvider::default()
-            );
-        }
-        let content = read_to_string(&config_path)
-            .with_context(|| format!("failed to read config at {}", config_path.display()))?;
-        let value: Value = serde_yaml::from_str(&content)
-            .with_context(|| format!("failed to parse config at {}", config_path.display()))?;
-
-        let provider = match value.get("secrets_provider") {
-            Some(v) if !v.is_null() => serde_yaml::from_value::<SupportedProvider>(v.clone())
-                .with_context(|| "failed to parse 'secrets_provider' from config")?,
-            _ => {
-                let password_file = value
-                    .get("vault_password_file")
-                    .and_then(|v| v.as_str())
-                    .map(PathBuf::from)
-                    .unwrap_or_else(|| AppConfig::default().vault_password_file());
-                SupportedProvider::Local {
-                    provider_def: LocalProvider {
-                        password_file: Some(password_file),
-                        git_branch: None,
-                        ..LocalProvider::default()
-                    },
-                }
-            }
         };
 
-        Ok(Self { provider })
-    }
-
-    pub fn default_local() -> Self {
         Self {
             provider: SupportedProvider::Local {
-                provider_def: LocalProvider {
+                provider_def: local_provider,
-                    password_file: Some(AppConfig::default().vault_password_file()),
-                    git_branch: None,
-                    ..LocalProvider::default()
-                },
             },
         }
     }
 
-    pub fn init(config: &AppConfig) -> Result<Self> {
+    pub fn init(config: &AppConfig) -> Self {
         let mut provider = match &config.secrets_provider {
             Some(p) => p.clone(),
             None => SupportedProvider::Local {
@@ -92,10 +55,11 @@ impl Vault {
         };
 
         if let SupportedProvider::Local { provider_def } = &mut provider {
-            ensure_password_file_initialized(provider_def)?;
+            ensure_password_file_initialized(provider_def)
+                .expect("Failed to initialize password file");
         }
 
-        Ok(Self { provider })
+        Self { provider }
     }
 
     pub fn local_password_file(&self) -> Result<PathBuf> {
@@ -213,7 +177,7 @@ impl Vault {
 
     pub fn validate_round_trip(&self) -> Result<()> {
         const PROBE_VALUE: &str = "ok";
-        let probe_key = format!("coyote-setup-probe-{}", Uuid::new_v4().simple());
+        let probe_key = format!("__coyote_setup_probe_{}__", Uuid::new_v4().simple());
 
         let h = Handle::current();
         let result: Result<()> = tokio::task::block_in_place(|| {
@@ -228,9 +192,7 @@ impl Vault {
                     .await
                     .with_context(|| "vault read probe failed")?;
                 if got != PROBE_VALUE {
-                    if let Err(cleanup_err) = self.provider_ref().delete_secret(&probe_key).await {
+                    let _ = self.provider_ref().delete_secret(&probe_key).await;
-                        warn!("vault probe cleanup failed for key '{probe_key}': {cleanup_err}");
-                    }
                     bail!("vault read probe returned an unexpected value");
                 }
 

src/vault/utils.rs

@@ -13,8 +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::PathBuf;
-use std::path::{Path, PathBuf};
 use std::process::Command;
 
 pub fn ensure_password_file_initialized(local_provider: &mut LocalProvider) -> Result<()> {
@@ -92,7 +91,6 @@ pub fn create_vault_password_file(vault: &mut Vault) -> Result<()> {
         match password {
             Ok(pw) => {
                 std::fs::write(&vault_password_file, pw.as_bytes())?;
-                set_password_file_permissions(&vault_password_file)?;
                 println!(
                     "✓ Password file '{}' updated.",
                     vault_password_file.display()
@@ -164,7 +162,6 @@ pub fn create_vault_password_file(vault: &mut Vault) -> Result<()> {
         match password {
             Ok(pw) => {
                 std::fs::write(&password_file, pw.as_bytes())?;
-                set_password_file_permissions(&password_file)?;
                 local_provider.password_file = Some(password_file);
                 println!(
                     "✓ Password file '{}' created.",
@@ -355,19 +352,6 @@ 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;
 
@@ -385,7 +369,7 @@ where
                     }
 
                     let name = caps[1].trim();
-                    match get_secret(name) {
+                    match vault.get_secret(name, false) {
                         Ok(s) => s,
                         Err(e) => match e.downcast_ref::<SecretError>() {
                             Some(SecretError::NotFound { .. }) => {
@@ -395,7 +379,7 @@ where
                             Some(SecretError::AuthFailed { .. }) => {
                                 let base =
                                     format!("Failed to fetch secret '{name}' from vault: {e}");
-                                let msg = match auth_hint {
+                                let msg = match vault.auth_hint() {
                                     Some(hint) => format!("{base}\n\nHint: {hint}"),
                                     None => base,
                                 };
@@ -422,199 +406,3 @@ where
 
     Ok((parsed_content, missing_secrets))
 }
-
-#[cfg(unix)]
-fn set_password_file_permissions(path: &Path) -> Result<()> {
-    use std::os::unix::fs::PermissionsExt;
-    std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).map_err(|e| {
-        anyhow!(
-            "Failed to set 0600 permissions on '{}': {e}",
-            path.display()
-        )
-    })
-}
-
-#[cfg(not(unix))]
-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 interpolates_multiple_secrets_on_same_line() {
-        let calls = Calls::new();
-
-        let (out, missing) = interpolate_secrets_with("url={{URL}} key={{KEY}}", None, |name| {
-            calls.record(name);
-            match name {
-                "URL" => Ok("https://example.test".to_string()),
-                "KEY" => Ok("sk-12345".to_string()),
-                other => panic!("unexpected lookup: {other}"),
-            }
-        })
-        .unwrap();
-
-        assert_eq!(calls.snapshot(), vec!["URL".to_string(), "KEY".to_string()]);
-        assert_eq!(out, "url=https://example.test key=sk-12345");
-        assert!(missing.is_empty());
-    }
-
-    #[test]
-    fn regex_rejects_braces_in_secret_names() {
-        let calls = Calls::new();
-
-        let (out, missing) =
-            interpolate_secrets_with("literal {{ {NOT_A_NAME} }} text", None, |name| {
-                calls.record(name);
-                Ok(format!("got-{name}"))
-            })
-            .unwrap();
-
-        assert!(
-            calls.snapshot().is_empty(),
-            "name with embedded braces must not match"
-        );
-        assert_eq!(out, "literal {{ {NOT_A_NAME} }} text");
-        assert!(missing.is_empty());
-    }
-
-    #[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}"
-        );
-    }
-}