refactor: improve build name validation, hook detection, and Windows compatibility

- Add Windows USERPROFILE fallback for HOME environment variable
- Enforce build name validation (no paths, no empty names)
- Refactor hook detection to separate builtins from custom hooks
- Extract custom hook commands from script shebangs
- Simplify struct initialization with default patterns
- Remove unnecessary dead_code annotations

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

Author: JheisonMB

src/builds/mod.rs

@@ -92,11 +92,17 @@ fn default_scope() -> String {
 }
 
 pub(crate) fn builds_dir() -> Result<PathBuf> {
-    let home = std::env::var("HOME").context("HOME environment variable not set")?;
+    let home = std::env::var("HOME")
+        .or_else(|_| std::env::var("USERPROFILE"))
+        .context("Neither HOME nor USERPROFILE environment variable is set")?;
     Ok(PathBuf::from(home).join(".gitkit").join("builds"))
 }
 
 fn build_path(name: &str) -> Result<PathBuf> {
+    anyhow::ensure!(
+        !name.is_empty() && !name.contains(['/', '\\']) && name != "." && name != "..",
+        "Invalid build name '{name}' — use a simple name without path separators"
+    );
     Ok(builds_dir()?.join(format!("{name}.toml")))
 }
 
@@ -256,7 +262,7 @@ pub(crate) fn apply_build(build: &Build) -> Result<()> {
     Ok(())
 }
 
-fn save(name: &str, description: Option<&str>) -> Result<()> {
+pub(crate) fn save(name: &str, description: Option<&str>) -> Result<()> {
     let path = build_path(name)?;
     if path.exists() {
         anyhow::bail!("Build '{name}' already exists. Delete it first or choose another name.");
@@ -278,21 +284,29 @@ pub(crate) fn capture_current_config(name: &str, description: Option<&str>) -> R
     let root = crate::utils::find_repo_root()?;
 
     let mut builtins = Vec::new();
+    let mut custom = Vec::new();
     let hooks_dir = root.join(".git").join("hooks");
     if hooks_dir.exists() {
         if let Ok(entries) = fs::read_dir(&hooks_dir) {
             for entry in entries.filter_map(|e| e.ok()) {
+                let path = entry.path();
+                if !path.is_file() {
+                    continue;
+                }
                 let hook_name = entry.file_name().to_string_lossy().to_string();
                 if hook_name.ends_with(".bak") || hook_name.ends_with(".sample") {
                     continue;
                 }
-                let content = fs::read_to_string(entry.path()).unwrap_or_default();
-                let prefix: String = content.chars().take(80).collect();
-                if let Some(b) = crate::hooks::available_builtins()
-                    .iter()
-                    .find(|b| b.hook == hook_name && content.contains(&prefix))
-                {
+                let content = fs::read_to_string(&path).unwrap_or_default();
+                if let Some(b) = crate::hooks::detect_builtin(&hook_name, &content) {
                     builtins.push(b.name.to_string());
+                } else if crate::hooks::valid_hook_names().contains(&hook_name.as_str()) {
+                    if let Some(command) = extract_custom_command(&content) {
+                        custom.push(CustomHook {
+                            hook: hook_name,
+                            command,
+                        });
+                    }
                 }
             }
         }
@@ -329,10 +343,7 @@ pub(crate) fn capture_current_config(name: &str, description: Option<&str>) -> R
     Ok(Build {
         name: name.to_string(),
         description: description.unwrap_or("").to_string(),
-        hooks: HooksConfig {
-            builtins,
-            custom: Vec::new(),
-        },
+        hooks: HooksConfig { builtins, custom },
         gitignore: GitignoreConfig { templates },
         gitattributes: GitattributesConfig { presets },
         config: ConfigBuild {
@@ -342,6 +353,23 @@ pub(crate) fn capture_current_config(name: &str, description: Option<&str>) -> R
     })
 }
 
+/// Recovers the command from a custom hook script (shebang + `set -e` + command).
+fn extract_custom_command(content: &str) -> Option<String> {
+    let lines: Vec<&str> = content
+        .lines()
+        .map(str::trim_end)
+        .filter(|l| {
+            let t = l.trim();
+            !t.is_empty() && !t.starts_with('#') && t != "set -e"
+        })
+        .collect();
+    if lines.is_empty() {
+        None
+    } else {
+        Some(lines.join("\n"))
+    }
+}
+
 fn detect_gitignore_templates(content: &str) -> Vec<String> {
     let mut templates = Vec::new();
 
@@ -381,7 +409,6 @@ fn delete(name: &str) -> Result<()> {
     Ok(())
 }
 
-#[allow(dead_code)]
 pub(crate) fn list_build_names() -> Vec<String> {
     builds_dir()
         .ok()
@@ -401,7 +428,6 @@ pub(crate) fn list_build_names() -> Vec<String> {
         .unwrap_or_default()
 }
 
-#[allow(dead_code)]
 pub(crate) fn load_build(name: &str) -> Result<Build> {
     let path = build_path(name)?;
     anyhow::ensure!(path.exists(), "Build '{name}' not found");
@@ -469,4 +495,23 @@ description = ""
         let presets = detect_gitattributes_presets(content);
         assert!(presets.contains(&"line-endings".to_string()));
     }
+
+    #[test]
+    fn extract_custom_command_recovers_command() {
+        let script = "#!/bin/sh\nset -e\ncargo test\n";
+        assert_eq!(extract_custom_command(script).as_deref(), Some("cargo test"));
+    }
+
+    #[test]
+    fn extract_custom_command_returns_none_for_empty_script() {
+        assert!(extract_custom_command("#!/bin/sh\nset -e\n").is_none());
+    }
+
+    #[test]
+    fn build_path_rejects_invalid_names() {
+        assert!(build_path("").is_err());
+        assert!(build_path("../evil").is_err());
+        assert!(build_path("a/b").is_err());
+        assert!(build_path("ok-name").is_ok());
+    }
 }

src/hooks/mod.rs

@@ -81,6 +81,17 @@ pub(crate) fn valid_hook_names() -> &'static [&'static str] {
     VALID_HOOKS
 }
 
+/// Identifies which built-in (if any) an installed hook file corresponds to,
+/// by exact script comparison. Built-ins are written verbatim on install.
+pub(crate) fn detect_builtin(
+    hook_file: &str,
+    content: &str,
+) -> Option<&'static builtins::Builtin> {
+    builtins::ALL
+        .iter()
+        .find(|b| b.hook == hook_file && content.trim() == b.script.trim())
+}
+
 fn hooks_dir() -> Result<std::path::PathBuf> {
     Ok(find_repo_root()?.join(".git").join("hooks"))
 }
@@ -294,4 +305,25 @@ mod tests {
         let err = resolve_hook("unknown-builtin", None).unwrap_err();
         assert!(err.to_string().contains("not a built-in"));
     }
+
+    #[test]
+    fn detect_builtin_distinguishes_builtins_sharing_a_hook_file() {
+        let no_secrets = builtins::get("no-secrets").unwrap();
+        let branch_naming = builtins::get("branch-naming").unwrap();
+        assert_eq!(
+            detect_builtin("pre-commit", no_secrets.script).map(|b| b.name),
+            Some("no-secrets")
+        );
+        assert_eq!(
+            detect_builtin("pre-commit", branch_naming.script).map(|b| b.name),
+            Some("branch-naming")
+        );
+    }
+
+    #[test]
+    fn detect_builtin_rejects_custom_scripts_and_wrong_hook() {
+        assert!(detect_builtin("pre-commit", "#!/bin/sh\nset -e\ncargo test\n").is_none());
+        let no_secrets = builtins::get("no-secrets").unwrap();
+        assert!(detect_builtin("commit-msg", no_secrets.script).is_none());
+    }
 }

src/init.rs

@@ -35,12 +35,10 @@ pub fn run() -> Result<()> {
         options.extend(saved_builds.iter().map(|b| format!("Use build: {b}")));
 
         let choice = Select::new("Saved builds available", options)
-            .with_help_message("↑↓ move  enter confirm")
-            .prompt_skippable()?
-            .unwrap_or_default();
+            .with_help_message("↑↓ move  enter confirm  esc start fresh")
+            .prompt_skippable()?;
 
-        if choice != "Start fresh configuration" {
-            let build_name = choice.strip_prefix("Use build: ").unwrap_or(&choice);
+        if let Some(build_name) = choice.as_deref().and_then(|c| c.strip_prefix("Use build: ")) {
             println!();
             let build = builds::load_build(build_name)?;
             builds::apply_build(&build)?;
@@ -97,10 +95,17 @@ pub fn run() -> Result<()> {
 
     for item in &hook_selections {
         if item == "Add custom hook..." {
-            let hook_name = Select::new("Hook type", hooks::valid_hook_names().to_vec())
-                .prompt()
-                .map_err(|e| anyhow::anyhow!("Hook selection cancelled: {}", e))?;
-            let command = Text::new("  Command to run").prompt()?;
+            let Some(hook_name) = Select::new("Hook type", hooks::valid_hook_names().to_vec())
+                .prompt_skippable()?
+            else {
+                continue;
+            };
+            let command = Text::new("  Command to run")
+                .prompt_skippable()?
+                .unwrap_or_default();
+            if command.trim().is_empty() {
+                continue;
+            }
             custom_hooks.push((hook_name.to_string(), command));
         } else if let Some(idx) = hook_items.iter().position(|i| i == item) {
             if idx < builtins.len() {
@@ -170,7 +175,7 @@ pub fn run() -> Result<()> {
     let defaults: Vec<usize> = config_options
         .iter()
         .enumerate()
-        .filter(|(_, o)| o.recommended)
+        .filter(|(_, o)| o.recommended || configured_keys.contains(o.key))
         .map(|(i, _)| i)
         .collect();
 
@@ -247,6 +252,16 @@ pub fn run() -> Result<()> {
     }
     for hook in &hooks_to_remove {
         if let Some(builtin) = hooks::available_builtins().iter().find(|b| b.name == *hook) {
+            // Several built-ins can share a hook file (e.g. pre-commit); don't
+            // delete the file if a freshly installed selection now owns it.
+            let file_reused = selected_builtins.iter().any(|sel| {
+                hooks::available_builtins()
+                    .iter()
+                    .any(|b| b.name == *sel && b.hook == builtin.hook)
+            });
+            if file_reused {
+                continue;
+            }
             if hooks::remove_hook(builtin.hook, true).is_ok() {
                 println!("  ◇ hook '{hook}' removed  ✓");
             }
@@ -269,20 +284,19 @@ pub fn run() -> Result<()> {
         )?;
         println!("  ◇ git config applied  ✓");
     }
+    // Only touch the repo's local config; a global value affects every repo,
+    // so it is never removed from here.
     for key in &configs_to_remove {
-        let removed = config::remove_config_key(key, config::ConfigScope::Local).is_ok()
-            || config::remove_config_key(key, config::ConfigScope::Global).is_ok();
-        if removed {
+        if config::remove_config_key(key, config::ConfigScope::Local).is_ok() {
             println!("  ◇ git config '{key}' removed  ✓");
+        } else {
+            println!(
+                "  ◇ git config '{key}' is set globally — left untouched (git config --global --unset {key})"
+            );
         }
     }
 
     // ── Save as build ─────────────────────────────────────────────────────
-    if nothing {
-        println!("\n  Nothing selected — exiting.");
-        return Ok(());
-    }
-
     println!();
     let save_build = inquire::Confirm::new("Save this configuration as a reusable build?")
         .with_default(false)
@@ -298,19 +312,9 @@ pub fn run() -> Result<()> {
         } else {
             Some(description.as_str())
         };
-        builds::capture_current_config(&name, desc_ref)
-            .and_then(|b| {
-                let dir = builds::builds_dir()?;
-                fs::create_dir_all(&dir)?;
-                let path = dir.join(format!("{name}.toml"));
-                let content = toml::to_string_pretty(&b)?;
-                fs::write(&path, content)?;
-                println!("  ✓ Build '{name}' saved");
-                Ok(())
-            })
-            .unwrap_or_else(|e| {
-                println!("  ⚠ Failed to save build: {e}");
-            });
+        if let Err(e) = builds::save(&name, desc_ref) {
+            println!("  ⚠ Failed to save build: {e}");
+        }
     }
 
     println!("\n  Done\n");
@@ -331,16 +335,7 @@ fn get_installed_hooks() -> HashSet<String> {
                     let name = entry.file_name().to_string_lossy().to_string();
                     if !name.ends_with(".bak") && !name.ends_with(".sample") {
                         let content = fs::read_to_string(entry.path()).unwrap_or_default();
-                        let builtin_match = hooks::available_builtins().iter().find(|b| {
-                            b.hook == name
-                                && b.script
-                                    .chars()
-                                    .take(80)
-                                    .collect::<String>()
-                                    .chars()
-                                    .all(|c| content.contains(c))
-                        });
-                        if let Some(b) = builtin_match {
+                        if let Some(b) = hooks::detect_builtin(&name, &content) {
                             installed.insert(b.name.to_string());
                         }
                     }
@@ -416,10 +411,11 @@ mod tests {
     use super::*;
 
     #[test]
-    fn get_configured_keys_works() {
+    fn get_configured_keys_only_returns_known_option_keys() {
         let configured = get_configured_keys();
-        // Just verify it doesn't panic and returns a valid HashSet
-        assert!(configured.len() >= 0);
+        for key in &configured {
+            assert!(config::CONFIG_OPTIONS.iter().any(|o| o.key == key));
+        }
     }
 
     #[test]

src/status/mod.rs

@@ -1,7 +1,6 @@
 use anyhow::Result;
 use std::fs;
 
-use crate::hooks::builtins;
 use crate::utils::{find_repo_root, git_config_get};
 
 pub fn run() -> Result<()> {
@@ -52,17 +51,7 @@ fn print_hooks() -> Result<()> {
         let hook_name = entry.file_name().to_string_lossy().to_string();
         let content = fs::read_to_string(entry.path()).unwrap_or_default();
 
-        let builtin_match = builtins::ALL.iter().find(|b| {
-            b.hook == hook_name
-                && b.script
-                    .chars()
-                    .take(80)
-                    .collect::<String>()
-                    .chars()
-                    .all(|c| content.contains(c))
-        });
-
-        match builtin_match {
+        match crate::hooks::detect_builtin(&hook_name, &content) {
             Some(b) => println!("  ✓ {} ({})", b.name, b.hook),
             None => {
                 let first_cmd = content
@@ -143,18 +132,10 @@ fn print_config(scope: &str) -> Result<()> {
         "--local"
     };
 
-    let configs = [
-        ("push.autoSetupRemote", "true"),
-        ("help.autocorrect", "prompt"),
-        ("diff.algorithm", "histogram"),
-        ("merge.conflictstyle", "zdiff3"),
-        ("rerere.enabled", "true"),
-    ];
-
     let mut any = false;
-    for (key, _expected) in &configs {
-        if let Some(value) = git_config_get(key, scope_flag) {
-            println!("  ✓ {key} = {value}");
+    for option in crate::config::CONFIG_OPTIONS {
+        if let Some(value) = git_config_get(option.key, scope_flag) {
+            println!("  ✓ {} = {value}", option.key);
             any = true;
         }
     }