fix(notfiles,nothooks): validate package on unlink; batch state I/O in HookRunner

Closes #10, closes #13

crates/notfiles/src/linker.rs

@@ -196,11 +196,21 @@ pub fn link_package(
 }
 
 pub fn unlink_package(
-    _dotfiles_dir: &Path,
+    dotfiles_dir: &Path,
     state: &mut State,
     package: &str,
     opts: &LinkOptions,
 ) -> Result<(), NotfilesError> {
+    // Validate the package: it must either exist as a directory in dotfiles_dir
+    // or have entries in state.  A name that satisfies neither is a user error.
+    let package_dir = dotfiles_dir.join(package);
+    let has_state_entries = !state.entries_for_package(package).is_empty();
+    if !package_dir.is_dir() && !has_state_entries {
+        return Err(NotfilesError::PackageNotFound {
+            name: package.to_string(),
+        });
+    }
+
     let entries: Vec<StateEntry> = state
         .entries_for_package(package)
         .into_iter()

crates/notgraph/src/emit.rs

@@ -172,19 +172,27 @@ fn write_html(
         .map(|fs| fs.name.as_str())
         .collect();
 
-    let mut leaves: Vec<&str> = crate_graph.nodes.iter()
+    let mut leaves: Vec<&str> = crate_graph
+        .nodes
+        .iter()
         .map(|n| n.as_str())
         .filter(|n| leaf_nodes.contains(n) && !root_nodes.contains(n))
         .collect();
-    let mut roots: Vec<&str> = crate_graph.nodes.iter()
+    let mut roots: Vec<&str> = crate_graph
+        .nodes
+        .iter()
         .map(|n| n.as_str())
         .filter(|n| root_nodes.contains(n) && !leaf_nodes.contains(n))
         .collect();
-    let mut features: Vec<&str> = crate_graph.nodes.iter()
+    let mut features: Vec<&str> = crate_graph
+        .nodes
+        .iter()
         .map(|n| n.as_str())
         .filter(|n| !leaf_nodes.contains(n) && !root_nodes.contains(n))
         .collect();
-    let mut isolated: Vec<&str> = crate_graph.nodes.iter()
+    let mut isolated: Vec<&str> = crate_graph
+        .nodes
+        .iter()
         .map(|n| n.as_str())
         .filter(|n| leaf_nodes.contains(n) && root_nodes.contains(n))
         .collect();
@@ -202,7 +210,11 @@ fn write_html(
         let syms = sym_count.get(n).copied().unwrap_or(0);
         format!(
             "{}[\"{}<br/>in:{} out:{} | {} pub\"]",
-            mermaid_id(n), n, fi, fo, syms
+            mermaid_id(n),
+            n,
+            fi,
+            fo,
+            syms
         )
     };
 
@@ -211,17 +223,23 @@ fn write_html(
 
     if !leaves.is_empty() {
         mermaid.push_str("  subgraph Core\n    direction TB\n");
-        for n in &leaves { mermaid.push_str(&format!("    {}\n", node_label(n))); }
+        for n in &leaves {
+            mermaid.push_str(&format!("    {}\n", node_label(n)));
+        }
         mermaid.push_str("  end\n");
     }
     if !features.is_empty() {
         mermaid.push_str("  subgraph Features\n    direction TB\n");
-        for n in &features { mermaid.push_str(&format!("    {}\n", node_label(n))); }
+        for n in &features {
+            mermaid.push_str(&format!("    {}\n", node_label(n)));
+        }
         mermaid.push_str("  end\n");
     }
     if !roots.is_empty() {
         mermaid.push_str("  subgraph Tools\n    direction TB\n");
-        for n in &roots { mermaid.push_str(&format!("    {}\n", node_label(n))); }
+        for n in &roots {
+            mermaid.push_str(&format!("    {}\n", node_label(n)));
+        }
         mermaid.push_str("  end\n");
     }
 
@@ -234,7 +252,8 @@ fn write_html(
         let color = heatmap_color(fs.fan_in);
         mermaid.push_str(&format!(
             "  style {} fill:{},stroke:#4a9eff,color:#e0e0e0\n",
-            mermaid_id(&fs.name), color
+            mermaid_id(&fs.name),
+            color
         ));
     }
 
@@ -316,10 +335,12 @@ fn write_html(
     let hotspot_rows: String = stats
         .hotspots
         .iter()
-        .map(|h| format!(
+        .map(|h| {
+            format!(
                 "<tr><td>{}</td><td>{}</td><td>{}</td></tr>",
                 h.name, h.kind, h.score
-        ))
+            )
+        })
         .collect::<Vec<_>>()
         .join("\n");
 

crates/notgraph/src/main.rs

@@ -63,7 +63,13 @@ fn main() -> Result<()> {
         workspace_root.join(&cli.output)
     };
 
-    emit::write_all(&output_dir, &crate_g, &module_graphs, &stats, &symbol_tables)
+    emit::write_all(
+        &output_dir,
+        &crate_g,
+        &module_graphs,
+        &stats,
+        &symbol_tables,
+    )
     .context("failed to write reports")?;
 
     println!("notgraph: reports written to {}", output_dir.display());

crates/nothooks/src/lib.rs

@@ -1,7 +1,7 @@
 pub mod runner;
 pub mod state;
 
-use notcore::{HookPhase, HookSpec, Report, StepStatus};
+use notcore::{HookPhase, HookSpec, Report};
 pub use runner::HookRunner;
 
 #[derive(Debug, PartialEq)]
@@ -12,16 +12,8 @@ pub enum HookResult {
 }
 
 /// Run all hooks matching `phase` and collect into a `Report`.
+///
+/// State is loaded once and saved once per call — not once per hook.
 pub fn run_phase(hooks: &[HookSpec], phase: &HookPhase, runner: &HookRunner) -> Report {
-    let mut report = Report::default();
+    runner.run_phase(hooks, phase)
-    for hook in hooks.iter().filter(|h| &h.phase == phase) {
-        let result = runner.run_hook(hook);
-        let status = match &result {
-            HookResult::Ok => StepStatus::Ok,
-            HookResult::Skipped => StepStatus::Skipped,
-            HookResult::Failed(msg) => StepStatus::Failed(msg.clone()),
-        };
-        report.add(&hook.name, status);
-    }
-    report
 }

crates/nothooks/src/runner.rs

@@ -1,6 +1,6 @@
 use crate::HookResult;
 use crate::state::HookState;
-use notcore::{HookPhase, HookSpec};
+use notcore::{HookPhase, HookSpec, Report, StepStatus};
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -46,9 +46,41 @@ impl HookRunner {
         }
     }
 
-    pub fn run_hook(&self, spec: &HookSpec) -> HookResult {
+    /// Run all hooks in `hooks` that match `phase`.
+    ///
+    /// State is loaded once before iterating and saved once at the end (only if
+    /// at least one Setup hook completed successfully), avoiding N file reads
+    /// and writes per phase.
+    pub fn run_phase(&self, hooks: &[HookSpec], phase: &HookPhase) -> Report {
         let mut state = HookState::load(&self.state_dir).unwrap_or_default();
+        let mut state_dirty = false;
+        let mut report = Report::default();
 
+        for spec in hooks.iter().filter(|h| &h.phase == phase) {
+            let result = self.run_hook_with_state(spec, &mut state, &mut state_dirty);
+            let status = match &result {
+                HookResult::Ok => StepStatus::Ok,
+                HookResult::Skipped => StepStatus::Skipped,
+                HookResult::Failed(msg) => StepStatus::Failed(msg.clone()),
+            };
+            report.add(&spec.name, status);
+        }
+
+        if state_dirty {
+            let _ = state.save(&self.state_dir);
+        }
+
+        report
+    }
+
+    /// Execute a single hook, using the caller-owned `state` and `dirty` flag
+    /// instead of loading/saving per invocation.
+    fn run_hook_with_state(
+        &self,
+        spec: &HookSpec,
+        state: &mut HookState,
+        state_dirty: &mut bool,
+    ) -> HookResult {
         if spec.phase == HookPhase::Setup && !self.force && state.is_done(&spec.name) {
             return HookResult::Skipped;
         }
@@ -63,7 +95,7 @@ impl HookRunner {
             Ok(status) if status.success() => {
                 if spec.phase == HookPhase::Setup {
                     state.mark_done(&spec.name);
-                    let _ = state.save(&self.state_dir);
+                    *state_dirty = true;
                 }
                 HookResult::Ok
             }
@@ -71,4 +103,18 @@ impl HookRunner {
             Err(e) => HookResult::Failed(e.to_string()),
         }
     }
+
+    /// Run a single hook with its own load/save cycle.
+    ///
+    /// Prefer [`HookRunner::run_phase`] when running multiple hooks to avoid
+    /// repeated state I/O.
+    pub fn run_hook(&self, spec: &HookSpec) -> HookResult {
+        let mut state = HookState::load(&self.state_dir).unwrap_or_default();
+        let mut state_dirty = false;
+        let result = self.run_hook_with_state(spec, &mut state, &mut state_dirty);
+        if state_dirty {
+            let _ = state.save(&self.state_dir);
+        }
+        result
+    }
 }

crates/notsecrets/src/sources/bitwarden.rs

@@ -50,12 +50,16 @@ impl BitwardenSource {
                     source: anyhow::anyhow!("bw unlock spawn: {e}"),
                 })?;
             if let Some(mut stdin) = child.stdin.take() {
-                stdin.write_all(password.as_bytes()).map_err(|e| AgeError::SourceError {
+                stdin
+                    .write_all(password.as_bytes())
+                    .map_err(|e| AgeError::SourceError {
                         name: self.name().to_string(),
                         source: anyhow::anyhow!("bw unlock stdin write: {e}"),
                     })?;
             }
-            let output = child.wait_with_output().map_err(|e| AgeError::SourceError {
+            let output = child
+                .wait_with_output()
+                .map_err(|e| AgeError::SourceError {
                     name: self.name().to_string(),
                     source: anyhow::anyhow!("bw unlock wait: {e}"),
                 })?;