From ed5e4102c9e659e37018faf0c09bb960129503ea Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Wed, 10 Sep 2025 07:51:03 +0100
Subject: [PATCH 01/12] Add structured errors for Ninja generation

---
 docs/netsuke-design.md                        | 46 ++++++++++---------
 docs/roadmap.md                               | 10 ++--
 ...snapshot-testing-in-netsuke-using-insta.md |  4 +-
 src/diagnostics.rs                            |  1 +
 src/manifest.rs                               | 34 ++++++--------
 src/ninja_gen.rs                              | 37 ++++++++++-----
 src/runner.rs                                 | 18 ++++----
 tests/ast_tests.rs                            |  2 +-
 tests/cucumber.rs                             |  1 +
 tests/features/ninja.feature                  |  6 +++
 tests/ninja_gen_tests.rs                      | 27 +++++++++--
 tests/ninja_snapshot_tests.rs                 |  2 +-
 tests/steps/ir_steps.rs                       | 16 +++++--
 tests/steps/ninja_steps.rs                    | 24 +++++++++-
 tests/yaml_error_tests.rs                     |  7 +--
 15 files changed, 148 insertions(+), 87 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index a83983c4..d64d30dc 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1181,17 +1181,17 @@ The command construction will follow this pattern:
 1. A new `Command` is created via `Command::new("ninja")`. Netsuke will assume
    `ninja` is available in the system's `PATH`.
 
-1. Arguments passed to Netsuke's own CLI will be translated and forwarded to
+2. Arguments passed to Netsuke's own CLI will be translated and forwarded to
    Ninja. For example, a `Netsuke build my_target` command would result in
    `Command::new("ninja").arg("my_target")`. Flags like `-j` for parallelism
    will also be passed through.[^8]
 
-1. The working directory for the Ninja process will be set using
+3. The working directory for the Ninja process will be set using
    `.current_dir()`. When the user supplies a `-C` flag, Netsuke canonicalises
    the path and applies it via `current_dir` rather than forwarding the flag to
    Ninja.
 
-1. Standard I/O streams (`stdin`, `stdout`, `stderr`) will be configured using
+4. Standard I/O streams (`stdin`, `stdout`, `stderr`) will be configured using
    `.stdout(Stdio::piped())` and `.stderr(Stdio::piped())`.[^24] This allows
    Netsuke to capture the real-time output from Ninja, which can then be
    streamed to the user's console, potentially with additional formatting or
@@ -1279,11 +1279,11 @@ three fundamental questions:
 1. **What** went wrong? A concise summary of the failure (e.g., "YAML parsing
    failed," "Build configuration is invalid").
 
-1. **Where** did it go wrong? Precise location information, including the file,
+2. **Where** did it go wrong? Precise location information, including the file,
    line number, and column where applicable (e.g., "in `Netsukefile` at line
    15, column 3").
 
-1. **Why** did it go wrong, and what can be done about it? The underlying cause
+3. **Why** did it go wrong, and what can be done about it? The underlying cause
    of the error and a concrete suggestion for how to fix it (e.g., "Cause:
    Found a tab character, which is not allowed. Hint: Use spaces for
    indentation instead.").
@@ -1346,23 +1346,26 @@ enrichment:
 
 1. A specific, low-level error occurs within a module. For instance, the IR
    generator detects a missing rule and creates an `IrGenError::RuleNotFound`.
+   Likewise, the Ninja generator returns `NinjaGenError::MissingAction` when a
+   build edge references an undefined action, preventing panics during file
+   generation.
 
-1. The function where the error occurred returns
+2. The function where the error occurred returns
    `Err(IrGenError::RuleNotFound {... }.into())`. The `.into()` call converts
    the specific `thiserror` enum variant into a generic `anyhow::Error` object,
    preserving the original error as its source.
 
-1. A higher-level function in the call stack, which called the failing function,
+3. A higher-level function in the call stack, which called the failing function,
    receives this `Err` value. It uses the `.with_context()` method to wrap the
    error with more application-level context. For example:
    `ir::from_manifest(ast)`
    `.with_context(|| "Failed to build the internal build graph from the manifest")?`
     .
 
-1. This process of propagation and contextualization repeats as the error
+4. This process of propagation and contextualization repeats as the error
    bubbles up towards `main`.
 
-1. Finally, the `main` function receives the `Err` result. It prints the entire
+5. Finally, the `main` function receives the `Err` result. It prints the entire
    error chain provided by `anyhow`, which displays the highest-level context
    first, followed by a list of underlying "Caused by:" messages. This provides
    the user with a rich, layered explanation of the failure, from the general
@@ -1530,15 +1533,15 @@ goal.
 
     1. Implement the initial `clap` CLI structure for the `build` command.
 
-    1. Implement the YAML parser using `serde_yml` and the AST data structures
+    2. Implement the YAML parser using `serde_yml` and the AST data structures
        (`ast.rs`).
 
-    1. Implement the AST-to-IR transformation logic, including basic validation
+    3. Implement the AST-to-IR transformation logic, including basic validation
        like checking for rule existence.
 
-    1. Implement the IR-to-Ninja file generator (`ninja_gen.rs`).
+    4. Implement the IR-to-Ninja file generator (`ninja_gen.rs`).
 
-    1. Implement the `std::process::Command` logic to invoke `ninja`.
+    5. Implement the `std::process::Command` logic to invoke `ninja`.
 
   - **Success Criterion:** Netsuke can successfully take a `Netsukefile` file
     *without any Jinja syntax* and compile it to a `build.ninja` file, then
@@ -1554,13 +1557,13 @@ goal.
 
     1. Integrate the `minijinja` crate into the build pipeline.
 
-    1. Implement the two-pass parsing mechanism: first render the manifest with
+    2. Implement the two-pass parsing mechanism: first render the manifest with
        `minijinja`, then parse the result with `serde_yml`.
 
-    1. Populate the initial Jinja context with the global `vars` from the
+    3. Populate the initial Jinja context with the global `vars` from the
        manifest.
 
-    1. Implement basic Jinja control flow (`{% if... %}`, `{% for... %}`) and
+    4. Implement basic Jinja control flow (`{% if... %}`, `{% for... %}`) and
        variable substitution.
 
   - **Success Criterion:** Netsuke can successfully build a manifest that uses
@@ -1577,15 +1580,15 @@ goal.
     1. Implement the full suite of custom Jinja functions (`glob`, `env`, etc.)
        and filters (`shell_escape`).
 
-    1. Mandate the use of `shell-quote` for all command variable substitutions.
+    2. Mandate the use of `shell-quote` for all command variable substitutions.
 
-    1. Refactor the error handling to fully adopt the `anyhow`/`thiserror`
+    3. Refactor the error handling to fully adopt the `anyhow`/`thiserror`
        strategy, ensuring all user-facing errors are contextual and actionable
        as specified in Section 7.
 
-    1. Implement the `clean` and `graph` subcommands.
+    4. Implement the `clean` and `graph` subcommands.
 
-    1. Refine the CLI output for clarity and readability.
+    5. Refine the CLI output for clarity and readability.
 
   - **Success Criterion:** Netsuke is a feature-complete, secure, and
     user-friendly build tool that meets all the initial design goals.
@@ -1652,7 +1655,8 @@ projects.
 ### **Works cited**
 
 [^1]: Ninja, a small build system with a focus on speed. Accessed on 12 July
-      2025. <https://ninja-build.org/>
+
+      1. <https://ninja-build.org/>
 
 [^2]: "Ninja (build system)." Wikipedia. Accessed on 12 July 2025.
       <https://en.wikipedia.org/wiki/Ninja_(build_system)>
diff --git a/docs/roadmap.md b/docs/roadmap.md
index 510c240b..0dc61bc3 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -138,18 +138,18 @@ library, and CLI ergonomics.
   - [x] After interpolation, validate the final command string is parsable using
     the shlex crate.
 
-- [ ] **Actionable Error Reporting:**
+- [x] **Actionable Error Reporting:**
 
-  - [ ] Adopt the `anyhow` and `thiserror` error handling strategy.
+  - [x] Adopt the `anyhow` and `thiserror` error handling strategy.
 
-  - [ ] Use thiserror to define specific, structured error types within library
+  - [x] Use thiserror to define specific, structured error types within library
 
     modules (e.g., IrGenError::RuleNotFound, IrGenError::CircularDependency).
 
-  - [ ] Use anyhow in the application logic to add human-readable context to
+  - [x] Use anyhow in the application logic to add human-readable context to
     errors as they propagate (e.g., using .with_context()).
 
-  - [ ] Refactor all error-producing code to provide the clear, contextual, and
+  - [x] Refactor all error-producing code to provide the clear, contextual, and
     actionable error messages specified in the design document.
 
 - [ ] **Template Standard Library:**
diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md
index c13ae619..b9206e11 100644
--- a/docs/snapshot-testing-in-netsuke-using-insta.md
+++ b/docs/snapshot-testing-in-netsuke-using-insta.md
@@ -176,7 +176,7 @@ fn simple_manifest_ninja_snapshot() {
     let build_graph = BuildGraph::from_manifest(&manifest).expect("IR generation succeeded");
 
     // Generate Ninja file content from the IR
-    let ninja_file = ninja_gen::generate_ninja(&build_graph)
+    let ninja_file = ninja_gen::generate(&build_graph)
         .expect("Ninja file generation succeeded");
 
     // The output is a multi-line Ninja build script (as a String)
@@ -196,7 +196,7 @@ Key points for Ninja snapshot tests:
   constructed directly for tests, but using the manifest→IR pipeline ensures
   realistic coverage.
 
-- Call the Ninja generation function (e.g. `ninja_gen::generate_ninja`), which
+- Call the Ninja generation function (e.g. `ninja_gen::generate`), which
   produces the Ninja file contents as a `String`. This function traverses the
   IR and outputs rules and build statements in Ninja syntax.
 
diff --git a/src/diagnostics.rs b/src/diagnostics.rs
index 3421e86b..dec61ffa 100644
--- a/src/diagnostics.rs
+++ b/src/diagnostics.rs
@@ -29,6 +29,7 @@ use std::fmt::Display;
 ///     File::open(path).diag("open file")
 /// }
 /// ```
+#[allow(dead_code, reason = "unused after migration to anyhow")]
 pub(crate) trait ResultExt<T> {
     /// Attach a static context message to any error.
     ///
diff --git a/src/manifest.rs b/src/manifest.rs
index 6fd99766..d5db5b00 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -7,11 +7,9 @@
 //! filesystem patterns during template evaluation. Both helpers fail fast when
 //! inputs are missing or patterns are invalid.
 
-use crate::{
-    ast::{NetsukeManifest, Recipe, StringOrList, Target, Vars},
-    diagnostics::ResultExt,
-};
-use miette::{Diagnostic, NamedSource, Report, Result, SourceSpan};
+use crate::ast::{NetsukeManifest, Recipe, StringOrList, Target, Vars};
+use anyhow::{Context, Result};
+use miette::{Diagnostic, NamedSource, SourceSpan};
 use minijinja::{Environment, Error, ErrorKind, UndefinedBehavior, context, value::Value};
 use serde_yml::{Error as YamlError, Location};
 use serde_yml::{Mapping as YamlMapping, Value as YamlValue};
@@ -531,7 +529,7 @@ fn glob_paths(pattern: &str) -> std::result::Result<Vec<String>, Error> {
 /// Returns an error if YAML parsing or Jinja evaluation fails.
 fn from_str_named(yaml: &str, name: &str) -> Result<NetsukeManifest> {
     let mut doc: YamlValue =
-        serde_yml::from_str(yaml).map_err(|e| Report::new(map_yaml_error(e, yaml, name)))?;
+        serde_yml::from_str(yaml).map_err(|e| map_yaml_error(e, yaml, name))?;
 
     let mut jinja = Environment::new();
     jinja.set_undefined_behavior(UndefinedBehavior::Strict);
@@ -543,7 +541,7 @@ fn from_str_named(yaml: &str, name: &str) -> Result<NetsukeManifest> {
         for (k, v) in vars {
             let key = k
                 .as_str()
-                .ok_or_else(|| miette::miette!("non-string key in 'vars' mapping: {k:?}"))?
+                .ok_or_else(|| anyhow::anyhow!("non-string key in vars mapping: {k:?}"))?
                 .to_string();
             jinja.add_global(key, Value::from_serialize(v));
         }
@@ -551,10 +549,8 @@ fn from_str_named(yaml: &str, name: &str) -> Result<NetsukeManifest> {
 
     expand_foreach(&mut doc, &jinja)?;
 
-    let manifest: NetsukeManifest = serde_yml::from_value(doc).map_err(|e| {
-        Report::new(ManifestError::Parse {
-            source: map_yaml_error(e, yaml, name),
-        })
+    let manifest: NetsukeManifest = serde_yml::from_value(doc).map_err(|e| ManifestError::Parse {
+        source: map_yaml_error(e, yaml, name),
     })?;
 
     render_manifest(manifest, &jinja)
@@ -618,7 +614,7 @@ fn parse_foreach_values(expr_val: &YamlValue, env: &Environment) -> Result<Vec<V
     let seq = eval_expression(env, "foreach", expr, context! {})?;
     let iter = seq
         .try_iter()
-        .diag("foreach expression did not yield an iterable")?;
+        .context("foreach expression did not yield an iterable")?;
     Ok(iter.collect())
 }
 
@@ -644,14 +640,14 @@ fn inject_iteration_vars(map: &mut YamlMapping, item: &Value, index: usize) -> R
         None => YamlMapping::new(),
         Some(YamlValue::Mapping(m)) => m,
         Some(other) => {
-            return Err(miette::miette!(
+            return Err(anyhow::anyhow!(
                 "target.vars must be a mapping, got: {other:?}"
             ));
         }
     };
     vars.insert(
         YamlValue::String("item".into()),
-        serde_yml::to_value(item).diag("serialise item")?,
+        serde_yml::to_value(item).context("serialise item")?,
     );
     vars.insert(
         YamlValue::String("index".into()),
@@ -664,14 +660,14 @@ fn inject_iteration_vars(map: &mut YamlMapping, item: &Value, index: usize) -> R
 fn as_str<'a>(value: &'a YamlValue, field: &str) -> Result<&'a str> {
     value
         .as_str()
-        .ok_or_else(|| miette::miette!("{field} must be a string expression"))
+        .ok_or_else(|| anyhow::anyhow!("{field} must be a string expression"))
 }
 
 fn eval_expression(env: &Environment, name: &str, expr: &str, ctx: Value) -> Result<Value> {
     env.compile_expression(expr)
-        .diag_with(|| format!("{name} expression parse error"))?
+        .with_context(|| format!("{name} expression parse error"))?
         .eval(ctx)
-        .diag_with(|| format!("{name} evaluation error"))
+        .with_context(|| format!("{name} evaluation error"))
 }
 
 /// Render a Jinja template and label any error with the given context.
@@ -681,7 +677,7 @@ fn render_str_with(
     ctx: &impl serde::Serialize,
     what: impl FnOnce() -> String,
 ) -> Result<String> {
-    env.render_str(tpl, ctx).diag_with(what)
+    env.render_str(tpl, ctx).with_context(what)
 }
 
 /// Render all templated strings in the manifest.
@@ -769,7 +765,7 @@ fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars
 pub fn from_path(path: impl AsRef<Path>) -> Result<NetsukeManifest> {
     let path_ref = path.as_ref();
     let data = fs::read_to_string(path_ref)
-        .diag_with(|| format!("failed to read {}", path_ref.display()))?;
+        .with_context(|| format!("failed to read {}", path_ref.display()))?;
     from_str_named(&data, &path_ref.display().to_string())
 }
 
diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index 906824b0..d12e68ef 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -10,6 +10,15 @@ use itertools::Itertools;
 use std::collections::HashSet;
 use std::fmt::{self, Display, Formatter, Write};
 use std::path::PathBuf;
+use thiserror::Error;
+
+#[derive(Debug, Error)]
+pub enum NinjaGenError {
+    #[error("action '{id}' referenced by build edge was not found")]
+    MissingAction { id: String },
+    #[error("failed to format Ninja output")]
+    Format(#[from] fmt::Error),
+}
 
 macro_rules! write_kv {
     ($f:expr, $key:expr, $opt:expr) => {
@@ -29,18 +38,17 @@ macro_rules! write_flag {
 
 /// Generate a Ninja build file as a string.
 ///
-/// # Panics
+/// # Errors
 ///
-/// Panics if a build edge references an unknown action or if writing to the
-/// output string fails (which is unexpected under normal conditions).
-#[must_use]
-pub fn generate(graph: &BuildGraph) -> String {
+/// Returns [`NinjaGenError`] if a build edge references an unknown action or
+/// writing to the output fails.
+pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
     let mut out = String::new();
 
     let mut actions: Vec<_> = graph.actions.iter().collect();
     actions.sort_by_key(|(id, _)| *id);
     for (id, action) in actions {
-        write!(out, "{}", NamedAction { id, action }).expect("write Ninja rule");
+        write!(out, "{}", NamedAction { id, action })?;
     }
 
     let mut edges: Vec<_> = graph.targets.values().collect();
@@ -51,7 +59,13 @@ pub fn generate(graph: &BuildGraph) -> String {
         if !seen.insert(key.clone()) {
             continue;
         }
-        let action = graph.actions.get(&edge.action_id).expect("action");
+        let action =
+            graph
+                .actions
+                .get(&edge.action_id)
+                .ok_or_else(|| NinjaGenError::MissingAction {
+                    id: edge.action_id.clone(),
+                })?;
         write!(
             out,
             "{}",
@@ -59,17 +73,16 @@ pub fn generate(graph: &BuildGraph) -> String {
                 edge,
                 action_restat: action.restat,
             }
-        )
-        .expect("write Ninja edge");
+        )?;
     }
 
     if !graph.default_targets.is_empty() {
         let mut defs = graph.default_targets.clone();
         defs.sort();
-        writeln!(out, "default {}", join(&defs)).expect("write defaults");
+        writeln!(out, "default {}", join(&defs))?;
     }
 
-    out
+    Ok(out)
 }
 
 /// Convert a slice of paths into a space-separated string.
@@ -195,7 +208,7 @@ mod tests {
         graph.targets.insert(PathBuf::from("out"), edge);
         graph.default_targets.push(PathBuf::from("out"));
 
-        let ninja = generate(&graph);
+        let ninja = generate(&graph).expect("generate ninja");
         let expected = concat!(
             "rule a\n",
             "  command = echo hi\n\n",
diff --git a/src/runner.rs b/src/runner.rs
index 09148250..38d3b009 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -5,9 +5,8 @@
 //! subprocess, streaming its output back to the user.
 
 use crate::cli::{BuildArgs, Cli, Commands};
-use crate::diagnostics::ResultExt;
 use crate::{ir::BuildGraph, manifest, ninja_gen};
-use miette::{Context, Result};
+use anyhow::{Context, Result};
 use serde_json;
 use std::borrow::Cow;
 use std::fs;
@@ -175,7 +174,7 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> {
     }
 
     let program = resolve_ninja_program();
-    run_ninja(program.as_path(), cli, build_path.as_ref(), &targets).diag("run ninja")?;
+    run_ninja(program.as_path(), cli, build_path.as_ref(), &targets).context("run ninja")?;
     drop(tmp_file);
     Ok(())
 }
@@ -197,7 +196,7 @@ fn create_temp_ninja_file(content: &NinjaContent) -> Result<NamedTempFile> {
         .prefix("netsuke.")
         .suffix(".ninja")
         .tempfile()
-        .diag("create temp file")?;
+        .context("create temp file")?;
     write_ninja_file(tmp.path(), content)?;
     Ok(tmp)
 }
@@ -218,10 +217,10 @@ fn write_ninja_file(path: &Path, content: &NinjaContent) -> Result<()> {
     // do not attempt to create the current directory on some platforms.
     if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) {
         fs::create_dir_all(parent)
-            .diag_with(|| format!("failed to create parent directory {}", parent.display()))?;
+            .with_context(|| format!("failed to create parent directory {}", parent.display()))?;
     }
     fs::write(path, content.as_str())
-        .diag_with(|| format!("failed to write Ninja file to {}", path.display()))?;
+        .with_context(|| format!("failed to write Ninja file to {}", path.display()))?;
     info!("Generated Ninja file at {}", path.display());
     Ok(())
 }
@@ -250,10 +249,11 @@ fn generate_ninja(cli: &Cli) -> Result<NinjaContent> {
     let manifest_path = resolve_manifest_path(cli);
     let manifest = manifest::from_path(&manifest_path)
         .with_context(|| format!("loading manifest at {}", manifest_path.display()))?;
-    let ast_json = serde_json::to_string_pretty(&manifest).diag("serialising manifest")?;
+    let ast_json = serde_json::to_string_pretty(&manifest).context("serialising manifest")?;
     debug!("AST:\n{ast_json}");
-    let graph = BuildGraph::from_manifest(&manifest).diag("building graph")?;
-    Ok(NinjaContent::new(ninja_gen::generate(&graph)))
+    let graph = BuildGraph::from_manifest(&manifest).context("building graph")?;
+    let ninja = ninja_gen::generate(&graph).context("generating Ninja file")?;
+    Ok(NinjaContent::new(ninja))
 }
 
 /// Determine the manifest path respecting the CLI's directory option.
diff --git a/tests/ast_tests.rs b/tests/ast_tests.rs
index 132c20ac..ced7ac6a 100644
--- a/tests/ast_tests.rs
+++ b/tests/ast_tests.rs
@@ -1,6 +1,6 @@
 //! Unit tests for Netsuke manifest AST deserialisation.
 
-use miette::Result;
+use anyhow::Result;
 use netsuke::{ast::*, manifest};
 use rstest::rstest;
 use semver::Version;
diff --git a/tests/cucumber.rs b/tests/cucumber.rs
index 422225ac..7b9da4fb 100644
--- a/tests/cucumber.rs
+++ b/tests/cucumber.rs
@@ -14,6 +14,7 @@ pub struct CliWorld {
     pub build_graph: Option<netsuke::ir::BuildGraph>,
     /// Generated Ninja file content.
     pub ninja: Option<String>,
+    pub ninja_error: Option<String>,
     /// Status of the last process execution (true for success, false for
     /// failure).
     pub run_status: Option<bool>,
diff --git a/tests/features/ninja.feature b/tests/features/ninja.feature
index e4d1dd39..ee7df53f 100644
--- a/tests/features/ninja.feature
+++ b/tests/features/ninja.feature
@@ -22,3 +22,9 @@ Feature: Ninja file generation
     And the ninja file is generated
     Then shlex splitting command 3 yields "printf, %s, -in file, >, o'utfile"
 
+  Scenario: Missing action is reported
+    When the manifest file "tests/data/rules.yml" is compiled to IR
+    And an action is removed from the graph
+    And the ninja file is generated
+    Then ninja generation fails with "referenced by build edge was not found"
+
diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs
index 2ddd5dd8..77f2ca28 100644
--- a/tests/ninja_gen_tests.rs
+++ b/tests/ninja_gen_tests.rs
@@ -8,7 +8,7 @@
 use insta::{Settings, assert_snapshot};
 use netsuke::ast::Recipe;
 use netsuke::ir::{Action, BuildEdge, BuildGraph};
-use netsuke::ninja_gen::generate;
+use netsuke::ninja_gen::{NinjaGenError, generate};
 use rstest::{fixture, rstest};
 use std::{fs, path::PathBuf, process::Command};
 use tempfile::{TempDir, tempdir};
@@ -131,14 +131,14 @@ fn generate_ninja_scenarios(
     graph.actions.insert(edge.action_id.clone(), action);
     graph.targets.insert(target_path, edge);
 
-    let ninja = generate(&graph);
+    let ninja = generate(&graph).expect("generate ninja");
     assert_eq!(ninja, expected);
 }
 
 #[rstest]
 fn generate_empty_graph() {
     let graph = BuildGraph::default();
-    let ninja = generate(&graph);
+    let ninja = generate(&graph).expect("generate ninja");
     assert!(ninja.is_empty());
 }
 
@@ -172,7 +172,7 @@ fn generate_multiline_script_snapshot() {
     );
     graph.default_targets.push(PathBuf::from("out"));
 
-    let ninja = generate(&graph);
+    let ninja = generate(&graph).expect("generate ninja");
     let mut settings = Settings::new();
     settings.set_snapshot_path(concat!(
         env!("CARGO_MANIFEST_DIR"),
@@ -299,7 +299,7 @@ fn ninja_integration_tests(
     graph.targets.insert(output.clone(), edge);
     graph.default_targets.push(output);
 
-    let ninja = generate(&graph);
+    let ninja = generate(&graph).expect("generate ninja");
     fs::write(dir.path().join("build.ninja"), &ninja).expect("write ninja");
     let status = Command::new("ninja")
         .args(&ninja_args)
@@ -321,3 +321,20 @@ fn ninja_integration_tests(
         }
     }
 }
+
+#[rstest]
+fn errors_when_action_missing() {
+    let mut graph = BuildGraph::default();
+    let edge = BuildEdge {
+        action_id: "missing".into(),
+        inputs: Vec::new(),
+        explicit_outputs: vec![PathBuf::from("out")],
+        implicit_outputs: Vec::new(),
+        order_only_deps: Vec::new(),
+        phony: false,
+        always: false,
+    };
+    graph.targets.insert(PathBuf::from("out"), edge);
+    let err = generate(&graph).expect_err("missing action");
+    assert!(matches!(err, NinjaGenError::MissingAction { id } if id == "missing"));
+}
diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs
index 10b3d60e..c80537d4 100644
--- a/tests/ninja_snapshot_tests.rs
+++ b/tests/ninja_snapshot_tests.rs
@@ -40,7 +40,7 @@ fn touch_manifest_ninja_validation() {
 
     let manifest = manifest::from_str(manifest_yaml).expect("parse manifest");
     let ir = BuildGraph::from_manifest(&manifest).expect("ir generation");
-    let ninja_content = ninja_gen::generate(&ir);
+    let ninja_content = ninja_gen::generate(&ir).expect("generate ninja");
 
     let mut settings = Settings::new();
     settings.set_snapshot_path(concat!(
diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs
index ceac21b9..9c7b6eec 100644
--- a/tests/steps/ir_steps.rs
+++ b/tests/steps/ir_steps.rs
@@ -2,7 +2,7 @@
 
 use crate::CliWorld;
 use cucumber::{given, then, when};
-use miette::{Context, IntoDiagnostic};
+use anyhow::Context;
 use netsuke::ir::BuildGraph;
 
 fn assert_graph(world: &CliWorld) {
@@ -52,9 +52,7 @@ fn graph_defaults(world: &mut CliWorld, count: usize) {
 fn compile_manifest(world: &mut CliWorld, path: String) {
     match netsuke::manifest::from_path(&path)
         .and_then(|m| {
-            BuildGraph::from_manifest(&m)
-                .into_diagnostic()
-                .wrap_err("building IR from manifest")
+            BuildGraph::from_manifest(&m).context("building IR from manifest")
         })
         .with_context(|| format!("IR generation failed for {path}"))
     {
@@ -88,6 +86,14 @@ fn generation_result_checked(world: &mut CliWorld) {
 fn ir_generation_fails(world: &mut CliWorld) {
     assert!(
         world.manifest_error.is_some(),
-        "expected IR generation error"
+        "expected IR generation error",
     );
 }
+
+#[when("an action is removed from the graph")]
+fn remove_action(world: &mut CliWorld) {
+    let graph = world.build_graph.as_mut().expect("graph");
+    if let Some(id) = graph.actions.keys().next().cloned() {
+        graph.actions.remove(&id);
+    }
+}
diff --git a/tests/steps/ninja_steps.rs b/tests/steps/ninja_steps.rs
index 6800fc4c..69e5f623 100644
--- a/tests/steps/ninja_steps.rs
+++ b/tests/steps/ninja_steps.rs
@@ -10,7 +10,16 @@ fn generate_ninja(world: &mut CliWorld) {
         .build_graph
         .as_ref()
         .expect("build graph should be available");
-    world.ninja = Some(ninja_gen::generate(graph));
+    match ninja_gen::generate(graph) {
+        Ok(n) => {
+            world.ninja = Some(n);
+            world.ninja_error = None;
+        }
+        Err(e) => {
+            world.ninja = None;
+            world.ninja_error = Some(e.to_string());
+        }
+    }
 }
 
 #[allow(
@@ -58,3 +67,16 @@ fn ninja_command_tokens(world: &mut CliWorld, index: usize, expected: String) {
 fn ninja_first_command_tokens(world: &mut CliWorld, expected: String) {
     ninja_command_tokens(world, 2, expected);
 }
+
+#[allow(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber requires owned String arguments"
+)]
+#[then(expr = "ninja generation fails with {string}")]
+fn ninja_generation_fails(world: &mut CliWorld, text: String) {
+    let err = world
+        .ninja_error
+        .as_ref()
+        .expect("ninja error should be available");
+    assert!(err.contains(&text));
+}
diff --git a/tests/yaml_error_tests.rs b/tests/yaml_error_tests.rs
index d783a834..6f4518c5 100644
--- a/tests/yaml_error_tests.rs
+++ b/tests/yaml_error_tests.rs
@@ -3,7 +3,6 @@
 //! These tests ensure diagnostics include line numbers and optional hints, and
 //! that rendering is stable across terminals.
 
-use miette::GraphicalReportHandler;
 use netsuke::manifest;
 use rstest::rstest;
 use strip_ansi_escapes::strip;
@@ -69,11 +68,7 @@ fn normalise_report(report: &str) -> String {
 )]
 fn yaml_diagnostics_are_actionable(#[case] yaml: &str, #[case] needles: &[&str]) {
     let err = manifest::from_str(yaml).expect_err("parse should fail");
-    let mut msg = String::new();
-    GraphicalReportHandler::new()
-        .render_report(&mut msg, err.as_ref())
-        .expect("render yaml error");
-    let msg = normalise_report(&msg);
+    let msg = normalise_report(&format!("{err:?}"));
     for needle in needles {
         assert!(msg.contains(needle), "missing: {needle}\nmessage: {msg}");
     }

From dce74dd9b89730ec30d2f80a4e5e7b7525e77ecb Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 11 Sep 2025 16:01:35 +0100
Subject: [PATCH 02/12] Tidy format error test

---
 src/ninja_gen.rs         | 11 ++++++++++-
 tests/ninja_gen_tests.rs | 41 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index d12e68ef..99905ca8 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -44,7 +44,16 @@ macro_rules! write_flag {
 /// writing to the output fails.
 pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
     let mut out = String::new();
+    generate_into(graph, &mut out)?;
+    Ok(out)
+}
 
+/// Write a Ninja build file to the provided writer.
+///
+/// # Errors
+///
+/// Returns [`NinjaGenError`] if a build edge references an unknown action or writing to the output fails.
+pub fn generate_into<W: Write>(graph: &BuildGraph, out: &mut W) -> Result<(), NinjaGenError> {
     let mut actions: Vec<_> = graph.actions.iter().collect();
     actions.sort_by_key(|(id, _)| *id);
     for (id, action) in actions {
@@ -82,7 +91,7 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
         writeln!(out, "default {}", join(&defs))?;
     }
 
-    Ok(out)
+    Ok(())
 }
 
 /// Convert a slice of paths into a space-separated string.
diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs
index 77f2ca28..58e6d7ec 100644
--- a/tests/ninja_gen_tests.rs
+++ b/tests/ninja_gen_tests.rs
@@ -8,7 +8,7 @@
 use insta::{Settings, assert_snapshot};
 use netsuke::ast::Recipe;
 use netsuke::ir::{Action, BuildEdge, BuildGraph};
-use netsuke::ninja_gen::{NinjaGenError, generate};
+use netsuke::ninja_gen::{NinjaGenError, generate, generate_into};
 use rstest::{fixture, rstest};
 use std::{fs, path::PathBuf, process::Command};
 use tempfile::{TempDir, tempdir};
@@ -338,3 +338,42 @@ fn errors_when_action_missing() {
     let err = generate(&graph).expect_err("missing action");
     assert!(matches!(err, NinjaGenError::MissingAction { id } if id == "missing"));
 }
+
+#[rstest]
+fn generate_format_error() {
+    use std::fmt::{self, Write};
+
+    struct FailWriter;
+    impl Write for FailWriter {
+        fn write_str(&mut self, _: &str) -> fmt::Result {
+            Err(fmt::Error)
+        }
+    }
+
+    let action = Action {
+        recipe: Recipe::Command {
+            command: "true".into(),
+        },
+        description: None,
+        depfile: None,
+        deps_format: None,
+        pool: None,
+        restat: false,
+    };
+    let edge = BuildEdge {
+        action_id: "a".into(),
+        inputs: Vec::new(),
+        explicit_outputs: vec![PathBuf::from("out")],
+        implicit_outputs: Vec::new(),
+        order_only_deps: Vec::new(),
+        phony: false,
+        always: false,
+    };
+    let mut graph = BuildGraph::default();
+    graph.actions.insert("a".into(), action);
+    graph.targets.insert(PathBuf::from("out"), edge);
+
+    let mut writer = FailWriter;
+    let err = generate_into(&graph, &mut writer).expect_err("format error");
+    assert!(matches!(err, NinjaGenError::Format(_)));
+}

From 5308e50cd7807582c7fd07cd6834445748eea035 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 11 Sep 2025 16:01:43 +0100
Subject: [PATCH 03/12] Format code and docs

---
 src/manifest.rs         | 7 ++++---
 tests/steps/ir_steps.rs | 6 ++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/manifest.rs b/src/manifest.rs
index d5db5b00..8c28376f 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -549,9 +549,10 @@ fn from_str_named(yaml: &str, name: &str) -> Result<NetsukeManifest> {
 
     expand_foreach(&mut doc, &jinja)?;
 
-    let manifest: NetsukeManifest = serde_yml::from_value(doc).map_err(|e| ManifestError::Parse {
-        source: map_yaml_error(e, yaml, name),
-    })?;
+    let manifest: NetsukeManifest =
+        serde_yml::from_value(doc).map_err(|e| ManifestError::Parse {
+            source: map_yaml_error(e, yaml, name),
+        })?;
 
     render_manifest(manifest, &jinja)
 }
diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs
index 9c7b6eec..66459fc5 100644
--- a/tests/steps/ir_steps.rs
+++ b/tests/steps/ir_steps.rs
@@ -1,8 +1,8 @@
 //! Step definitions for `BuildGraph` scenarios.
 
 use crate::CliWorld;
-use cucumber::{given, then, when};
 use anyhow::Context;
+use cucumber::{given, then, when};
 use netsuke::ir::BuildGraph;
 
 fn assert_graph(world: &CliWorld) {
@@ -51,9 +51,7 @@ fn graph_defaults(world: &mut CliWorld, count: usize) {
 #[when(expr = "the manifest file {string} is compiled to IR")]
 fn compile_manifest(world: &mut CliWorld, path: String) {
     match netsuke::manifest::from_path(&path)
-        .and_then(|m| {
-            BuildGraph::from_manifest(&m).context("building IR from manifest")
-        })
+        .and_then(|m| BuildGraph::from_manifest(&m).context("building IR from manifest"))
         .with_context(|| format!("IR generation failed for {path}"))
     {
         Ok(graph) => {

From e530bdddad387b8af0f2923ec8241af07c6aa309 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Fri, 12 Sep 2025 03:22:46 +0100
Subject: [PATCH 04/12] Clarify miette's role in error handling

---
 docs/netsuke-design.md | 94 +++++++++++++++++++++++++++++++++---------
 docs/roadmap.md        |  2 +
 2 files changed, 76 insertions(+), 20 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index d64d30dc..a4c4ed91 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1290,16 +1290,21 @@ three fundamental questions:
 
 ### 7.2 Crate Selection and Strategy: `anyhow`, `thiserror`, and `miette`
 
-To implement this philosophy, Netsuke adopts a hybrid error handling strategy
-using the `anyhow`, `thiserror`, and `miette` crates. This is a common and
-highly effective pattern in the Rust ecosystem for creating robust applications
-and libraries.[^27] `miette` renders user-facing diagnostics, computing spans
-directly from parser locations.
+Netsuke uses a two-tier error architecture:
 
-- `thiserror`: This crate will be used *within* Netsuke's internal library
-  modules (e.g., `parser`, `ir`, `ninja_gen`) to define specific, structured
-  error types. The `#[derive(Error)]` macro reduces boilerplate and allows for
-  the creation of rich, semantic errors.[^29]
+1. `anyhow` captures internal context as errors propagate through the
+   application.
+2. `miette` renders user-facing diagnostics and **is not optional**. All
+   surface errors must implement `miette::Diagnostic` so the CLI can present
+   spans, annotated source, and helpful suggestions.
+
+This hybrid strategy is common in the Rust ecosystem and provides both rich
+context and polished user output.[^27]
+
+- `thiserror`: This crate is used *within* Netsuke's internal library modules
+  (e.g., `parser`, `ir`, `ninja_gen`) to define specific, structured error
+  types. The `#[derive(Error)]` macro reduces boilerplate and allows for the
+  creation of rich, semantic errors.[^29]
 
 Rust
 
@@ -1327,17 +1332,64 @@ pub enum IrGenError {
     ActionSerialisation(#[from] serde_json::Error), }
 ```
 
-- `anyhow`: This crate will be used in the main application logic (`main.rs`)
-  and at the boundaries between modules. `anyhow::Result` serves as a
-  convenient, dynamic error type that can wrap any underlying error that
-  implements `std::error::Error`.[^30] The primary tools used will be the
-
-  `?` operator for clean error propagation and the `.context()` and
-  `.with_context()` methods for adding high-level, human-readable context to
-  errors as they bubble up the call stack.[^31]
+- `anyhow`: Used in the main application logic (`main.rs`) and at the
+  boundaries between modules. `anyhow::Result` wraps any error implementing
+  `std::error::Error`.[^30] The `?` operator provides clean propagation, while
+  `.context()` and `.with_context()` attach high-level explanations as errors
+  bubble up.[^31]
 
 - `miette`: Presents human-friendly diagnostics, highlighting exact error
-  locations with computed spans.
+  locations with computed spans. Every diagnostic must retain `miette`'s
+  `Diagnostic` implementation as it travels through `anyhow`.
+
+#### Canonical pattern: `YamlDiagnostic`
+
+`YamlDiagnostic` is the reference implementation of a Netsuke diagnostic. It
+wraps `yaml-rust` errors with annotated source, spans, and optional help text:
+
+```rust
+#[derive(Debug, Error, Diagnostic)]
+#[error("{message}")]
+#[diagnostic(code(netsuke::yaml::parse))]
+pub struct YamlDiagnostic {
+    #[source_code]
+    src: NamedSource<String>,
+    #[label("parse error here")]
+    span: Option<SourceSpan>,
+    #[help]
+    help: Option<String>,
+    #[source]
+    source: YamlError,
+    message: String,
+}
+
+#[derive(Debug, Error, Diagnostic)]
+pub enum ManifestError {
+    #[error("manifest parse error")]
+    #[diagnostic(code(netsuke::manifest::parse))]
+    Parse {
+        #[source]
+        #[diagnostic_source]
+        source: YamlDiagnostic,
+    },
+}
+```
+
+`ManifestError::Parse` preserves the rich diagnostic, allowing `miette` to show
+the offending YAML snippet. All new user-facing errors with source context must
+follow this model.
+
+Common use cases requiring `miette` diagnostics include:
+
+- YAML parsing errors.
+- Jinja template rendering failures with line numbers and context.
+- Any scenario where highlighting spans or providing structured help benefits
+  the user.
+
+Although `src/diagnostics.rs` is currently unused, it contains prototypes for
+`miette` patterns and remains a valuable reference. Future diagnostics should
+mirror the `YamlDiagnostic` approach by implementing `Diagnostic`, providing a
+`NamedSource`, a `SourceSpan`, and actionable help text.
 
 ### 7.3 Error Handling Flow
 
@@ -1362,8 +1414,10 @@ enrichment:
    `.with_context(|| "Failed to build the internal build graph from the manifest")?`
     .
 
-4. This process of propagation and contextualization repeats as the error
-   bubbles up towards `main`.
+4. This process of propagation and contextualisation repeats as the error
+   bubbles up towards `main`. Use `anyhow::Context` to add detail, but never
+   convert a `miette::Diagnostic` into a plain `anyhow::Error`—doing so would
+   discard spans and help text.
 
 5. Finally, the `main` function receives the `Err` result. It prints the entire
    error chain provided by `anyhow`, which displays the highest-level context
diff --git a/docs/roadmap.md b/docs/roadmap.md
index 0dc61bc3..6dbbf41e 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -148,6 +148,8 @@ library, and CLI ergonomics.
 
   - [x] Use anyhow in the application logic to add human-readable context to
     errors as they propagate (e.g., using .with_context()).
+  - [x] Use `miette` to render diagnostics with source spans and helpful
+    messages.
 
   - [x] Refactor all error-producing code to provide the clear, contextual, and
     actionable error messages specified in the design document.

From 24a15671a8a08fef48f3d0a82fec914e8aeb9e52 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Fri, 12 Sep 2025 03:22:51 +0100
Subject: [PATCH 05/12] Split runner process helpers

---
 src/manifest.rs           |   2 +-
 src/runner.rs             | 196 +-------------------------------------
 src/runner/process.rs     | 196 ++++++++++++++++++++++++++++++++++++++
 tests/ast_tests.rs        |   2 +-
 tests/yaml_error_tests.rs |   7 +-
 5 files changed, 207 insertions(+), 196 deletions(-)
 create mode 100644 src/runner/process.rs

diff --git a/src/manifest.rs b/src/manifest.rs
index 8c28376f..484b583e 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -36,7 +36,7 @@ fn to_span(src: &str, loc: Location) -> SourceSpan {
         }
     };
     let len = end.saturating_sub(start);
-    #[allow(clippy::useless_conversion, reason = "future-proof span length type")]
+    #[expect(clippy::useless_conversion, reason = "future-proof span length type")]
     SourceSpan::new(start.into(), len.into())
 }
 
diff --git a/src/runner.rs b/src/runner.rs
index 38d3b009..c0ea5dc9 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -10,10 +10,7 @@ use anyhow::{Context, Result};
 use serde_json;
 use std::borrow::Cow;
 use std::fs;
-use std::io::{self, BufRead, BufReader, Write};
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
-use std::thread;
 use tempfile::{Builder, NamedTempFile};
 use tracing::{debug, info};
 
@@ -22,31 +19,11 @@ pub const NINJA_PROGRAM: &str = "ninja";
 /// Environment variable override for the Ninja executable.
 pub use ninja_env::NINJA_ENV;
 
-// Public helpers for doctests only. This exposes internal helpers as a stable
-// testing surface without exporting them in release builds.
+mod process;
 #[cfg(doctest)]
 #[doc(hidden)]
-pub mod doc {
-    pub use super::CommandArg;
-
-    // Public wrappers to expose crate-private helpers to doctests.
-    #[must_use]
-    pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool {
-        super::contains_sensitive_keyword(arg)
-    }
-    #[must_use]
-    pub fn is_sensitive_arg(arg: &CommandArg) -> bool {
-        super::is_sensitive_arg(arg)
-    }
-    #[must_use]
-    pub fn redact_argument(arg: &CommandArg) -> CommandArg {
-        super::redact_argument(arg)
-    }
-    #[must_use]
-    pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec<CommandArg> {
-        super::redact_sensitive_args(args)
-    }
-}
+pub use process::doc;
+pub use process::run_ninja;
 
 #[derive(Debug, Clone)]
 pub struct NinjaContent(String);
@@ -65,19 +42,6 @@ impl NinjaContent {
     }
 }
 
-#[derive(Debug, Clone)]
-pub struct CommandArg(String);
-impl CommandArg {
-    #[must_use]
-    pub fn new(arg: String) -> Self {
-        Self(arg)
-    }
-    #[must_use]
-    pub fn as_str(&self) -> &str {
-        &self.0
-    }
-}
-
 /// Target list passed through to Ninja.
 /// An empty slice means “use the defaults” emitted by IR generation
 /// (default targets).
@@ -277,157 +241,3 @@ fn resolve_manifest_path(cli: &Cli) -> std::path::PathBuf {
 fn resolve_ninja_program() -> PathBuf {
     std::env::var_os(NINJA_ENV).map_or_else(|| PathBuf::from(NINJA_PROGRAM), PathBuf::from)
 }
-
-/// Check if `arg` contains a sensitive keyword.
-///
-/// # Examples
-/// ```
-/// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword};
-/// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into())));
-/// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into())));
-/// ```
-pub(crate) fn contains_sensitive_keyword(arg: &CommandArg) -> bool {
-    let lower = arg.as_str().to_lowercase();
-    lower.contains("password") || lower.contains("token") || lower.contains("secret")
-}
-
-/// Determine whether the argument should be redacted.
-///
-/// # Examples
-/// ```
-/// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg};
-/// assert!(is_sensitive_arg(&CommandArg::new("password=123".into())));
-/// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into())));
-/// ```
-pub(crate) fn is_sensitive_arg(arg: &CommandArg) -> bool {
-    contains_sensitive_keyword(arg)
-}
-
-/// Redact sensitive information in a single argument.
-///
-/// Sensitive values are replaced with `***REDACTED***`, preserving keys.
-///
-/// # Examples
-/// ```
-/// # use netsuke::runner::doc::{CommandArg, redact_argument};
-/// let arg = CommandArg::new("token=abc".into());
-/// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***");
-/// let arg = CommandArg::new("path=/tmp".into());
-/// assert_eq!(redact_argument(&arg).as_str(), "path=/tmp");
-/// ```
-pub(crate) fn redact_argument(arg: &CommandArg) -> CommandArg {
-    if is_sensitive_arg(arg) {
-        let redacted = arg.as_str().split_once('=').map_or_else(
-            || "***REDACTED***".to_string(),
-            |(key, _)| format!("{key}=***REDACTED***"),
-        );
-        CommandArg::new(redacted)
-    } else {
-        arg.clone()
-    }
-}
-
-/// Redact sensitive information from all `args`.
-///
-/// # Examples
-/// ```
-/// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args};
-/// let args = vec![
-///     CommandArg::new("ninja".into()),
-///     CommandArg::new("token=abc".into()),
-/// ];
-/// let redacted = redact_sensitive_args(&args);
-/// assert_eq!(redacted[1].as_str(), "token=***REDACTED***");
-/// ```
-pub(crate) fn redact_sensitive_args(args: &[CommandArg]) -> Vec<CommandArg> {
-    args.iter().map(redact_argument).collect()
-}
-
-/// Invoke the Ninja executable with the provided CLI settings.
-///
-/// The function forwards the job count and working directory to Ninja,
-/// specifies the temporary build file, and streams its standard output and
-/// error back to the user.
-///
-/// # Errors
-///
-/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a
-/// non-zero exit status.
-///
-/// # Panics
-///
-/// Panics if the child's output streams cannot be captured.
-pub fn run_ninja(
-    program: &Path,
-    cli: &Cli,
-    build_file: &Path,
-    targets: &BuildTargets<'_>,
-) -> io::Result<()> {
-    let mut cmd = Command::new(program);
-    if let Some(dir) = &cli.directory {
-        // Resolve and canonicalise the directory so Ninja receives a stable
-        // absolute path. Using only `current_dir` avoids combining it with
-        // Ninja's own `-C` flag which would otherwise double-apply the
-        // directory and break relative paths.
-        let dir = fs::canonicalize(dir)?;
-        cmd.current_dir(dir);
-    }
-    if let Some(jobs) = cli.jobs {
-        cmd.arg("-j").arg(jobs.to_string());
-    }
-    // Canonicalise the build file path so Ninja resolves it correctly from the
-    // working directory. Fall back to the original on failure so Ninja can
-    // surface a meaningful error.
-    let build_file_path = build_file
-        .canonicalize()
-        .unwrap_or_else(|_| build_file.to_path_buf());
-    cmd.arg("-f").arg(&build_file_path);
-    cmd.args(targets.as_slice());
-    cmd.stdout(Stdio::piped());
-    cmd.stderr(Stdio::piped());
-
-    let program = cmd.get_program().to_string_lossy().into_owned();
-    let args: Vec<CommandArg> = cmd
-        .get_args()
-        .map(|a| CommandArg::new(a.to_string_lossy().into_owned()))
-        .collect();
-    let redacted_args = redact_sensitive_args(&args);
-    let arg_strings: Vec<&str> = redacted_args.iter().map(CommandArg::as_str).collect();
-    info!("Running command: {} {}", program, arg_strings.join(" "));
-
-    let mut child = cmd.spawn()?;
-    let stdout = child.stdout.take().expect("child stdout");
-    let stderr = child.stderr.take().expect("child stderr");
-
-    let out_handle = thread::spawn(move || {
-        let reader = BufReader::new(stdout);
-        let mut handle = io::stdout();
-        for line in reader.lines().map_while(Result::ok) {
-            let _ = writeln!(handle, "{line}");
-        }
-    });
-    let err_handle = thread::spawn(move || {
-        let reader = BufReader::new(stderr);
-        let mut handle = io::stderr();
-        for line in reader.lines().map_while(Result::ok) {
-            let _ = writeln!(handle, "{line}");
-        }
-    });
-
-    let status = child.wait()?;
-    let _ = out_handle.join();
-    let _ = err_handle.join();
-
-    if status.success() {
-        Ok(())
-    } else {
-        #[expect(
-            clippy::io_other_error,
-            reason = "use explicit error kind for compatibility with older Rust"
-        )]
-        Err(io::Error::new(
-            io::ErrorKind::Other,
-            format!("ninja exited with {status}"),
-        ))
-    }
-}
diff --git a/src/runner/process.rs b/src/runner/process.rs
new file mode 100644
index 00000000..b4a6f187
--- /dev/null
+++ b/src/runner/process.rs
@@ -0,0 +1,196 @@
+use super::BuildTargets;
+use crate::cli::Cli;
+use std::{
+    fs,
+    io::{self, BufRead, BufReader, Write},
+    path::Path,
+    process::{Command, Stdio},
+    thread,
+};
+use tracing::info;
+
+#[derive(Debug, Clone)]
+pub struct CommandArg(String);
+impl CommandArg {
+    #[must_use]
+    pub fn new(arg: String) -> Self {
+        Self(arg)
+    }
+    #[must_use]
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+}
+
+// Public helpers for doctests only. This exposes internal helpers as a stable
+// testing surface without exporting them in release builds.
+#[cfg(doctest)]
+#[doc(hidden)]
+pub mod doc {
+    pub use super::CommandArg;
+
+    #[must_use]
+    pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool {
+        super::contains_sensitive_keyword(arg)
+    }
+    #[must_use]
+    pub fn is_sensitive_arg(arg: &CommandArg) -> bool {
+        super::is_sensitive_arg(arg)
+    }
+    #[must_use]
+    pub fn redact_argument(arg: &CommandArg) -> CommandArg {
+        super::redact_argument(arg)
+    }
+    #[must_use]
+    pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec<CommandArg> {
+        super::redact_sensitive_args(args)
+    }
+}
+
+/// Check if `arg` contains a sensitive keyword.
+///
+/// # Examples
+/// ```
+/// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword};
+/// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into())));
+/// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into())));
+/// ```
+pub(crate) fn contains_sensitive_keyword(arg: &CommandArg) -> bool {
+    let lower = arg.as_str().to_lowercase();
+    lower.contains("password") || lower.contains("token") || lower.contains("secret")
+}
+
+/// Determine whether the argument should be redacted.
+/// Determine whether the argument should be redacted.
+///
+/// # Examples
+/// ```
+/// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg};
+/// assert!(is_sensitive_arg(&CommandArg::new("password=123".into())));
+/// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into())));
+/// ```
+pub(crate) fn is_sensitive_arg(arg: &CommandArg) -> bool {
+    contains_sensitive_keyword(arg)
+}
+
+/// Redact sensitive information in a single argument.
+///
+/// Sensitive values are replaced with `***REDACTED***`, preserving keys.
+///
+/// # Examples
+/// ```
+/// # use netsuke::runner::doc::{CommandArg, redact_argument};
+/// let arg = CommandArg::new("token=abc".into());
+/// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***");
+/// let arg = CommandArg::new("path=/tmp".into());
+/// assert_eq!(redact_argument(&arg).as_str(), "path=/tmp");
+/// ```
+pub(crate) fn redact_argument(arg: &CommandArg) -> CommandArg {
+    if is_sensitive_arg(arg) {
+        let redacted = arg.as_str().split_once('=').map_or_else(
+            || "***REDACTED***".to_string(),
+            |(key, _)| format!("{key}=***REDACTED***"),
+        );
+        CommandArg::new(redacted)
+    } else {
+        arg.clone()
+    }
+}
+
+/// Redact sensitive information from all `args`.
+///
+/// # Examples
+/// ```
+/// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args};
+/// let args = vec![
+///     CommandArg::new("ninja".into()),
+///     CommandArg::new("token=abc".into()),
+/// ];
+/// let redacted = redact_sensitive_args(&args);
+/// assert_eq!(redacted[1].as_str(), "token=***REDACTED***");
+/// ```
+pub(crate) fn redact_sensitive_args(args: &[CommandArg]) -> Vec<CommandArg> {
+    args.iter().map(redact_argument).collect()
+}
+
+/// Invoke the Ninja executable with the provided CLI settings.
+///
+/// The function forwards the job count and working directory to Ninja,
+/// specifies the temporary build file, and streams its standard output and
+/// error back to the user.
+///
+/// # Errors
+///
+/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a
+/// non-zero exit status.
+///
+/// # Panics
+///
+/// Panics if the child's output streams cannot be captured.
+pub fn run_ninja(
+    program: &Path,
+    cli: &Cli,
+    build_file: &Path,
+    targets: &BuildTargets<'_>,
+) -> io::Result<()> {
+    let mut cmd = Command::new(program);
+    if let Some(dir) = &cli.directory {
+        let dir = fs::canonicalize(dir)?;
+        cmd.current_dir(dir);
+    }
+    if let Some(jobs) = cli.jobs {
+        cmd.arg("-j").arg(jobs.to_string());
+    }
+    let build_file_path = build_file
+        .canonicalize()
+        .unwrap_or_else(|_| build_file.to_path_buf());
+    cmd.arg("-f").arg(&build_file_path);
+    cmd.args(targets.as_slice());
+    cmd.stdout(Stdio::piped());
+    cmd.stderr(Stdio::piped());
+
+    let program = cmd.get_program().to_string_lossy().into_owned();
+    let args: Vec<CommandArg> = cmd
+        .get_args()
+        .map(|a| CommandArg::new(a.to_string_lossy().into_owned()))
+        .collect();
+    let redacted_args = redact_sensitive_args(&args);
+    let arg_strings: Vec<&str> = redacted_args.iter().map(CommandArg::as_str).collect();
+    info!("Running command: {} {}", program, arg_strings.join(" "));
+
+    let mut child = cmd.spawn()?;
+    let stdout = child.stdout.take().expect("child stdout");
+    let stderr = child.stderr.take().expect("child stderr");
+
+    let out_handle = thread::spawn(move || {
+        let reader = BufReader::new(stdout);
+        let mut handle = io::stdout();
+        for line in reader.lines().map_while(Result::ok) {
+            let _ = writeln!(handle, "{line}");
+        }
+    });
+    let err_handle = thread::spawn(move || {
+        let reader = BufReader::new(stderr);
+        let mut handle = io::stderr();
+        for line in reader.lines().map_while(Result::ok) {
+            let _ = writeln!(handle, "{line}");
+        }
+    });
+
+    let status = child.wait()?;
+    let _ = out_handle.join();
+    let _ = err_handle.join();
+
+    if status.success() {
+        Ok(())
+    } else {
+        #[expect(
+            clippy::io_other_error,
+            reason = "use explicit error kind for compatibility with older Rust"
+        )]
+        Err(io::Error::new(
+            io::ErrorKind::Other,
+            format!("ninja exited with {status}"),
+        ))
+    }
+}
diff --git a/tests/ast_tests.rs b/tests/ast_tests.rs
index ced7ac6a..19ea6844 100644
--- a/tests/ast_tests.rs
+++ b/tests/ast_tests.rs
@@ -291,7 +291,7 @@ fn phony_and_always_flags() {
     true,
     true
 )]
-fn actions_behavior(
+fn actions_behaviour(
     #[case] yaml: &str,
     #[case] expected_phony: bool,
     #[case] expected_always: bool,
diff --git a/tests/yaml_error_tests.rs b/tests/yaml_error_tests.rs
index 6f4518c5..4f1ad259 100644
--- a/tests/yaml_error_tests.rs
+++ b/tests/yaml_error_tests.rs
@@ -68,7 +68,12 @@ fn normalise_report(report: &str) -> String {
 )]
 fn yaml_diagnostics_are_actionable(#[case] yaml: &str, #[case] needles: &[&str]) {
     let err = manifest::from_str(yaml).expect_err("parse should fail");
-    let msg = normalise_report(&format!("{err:?}"));
+    let msg = normalise_report(
+        &err.chain()
+            .map(ToString::to_string)
+            .collect::<Vec<_>>()
+            .join("\n"),
+    );
     for needle in needles {
         assert!(msg.contains(needle), "missing: {needle}\nmessage: {msg}");
     }

From a7902516958865b40f6659a5c3bbddfae4e84acd Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Fri, 12 Sep 2025 12:51:16 +0100
Subject: [PATCH 06/12] Document Ninja generator and strengthen error handling

---
 ...snapshot-testing-in-netsuke-using-insta.md |  8 ++--
 src/diagnostics.rs                            |  2 +-
 src/manifest.rs                               |  5 ++-
 src/ninja_gen.rs                              | 43 +++++++++++++++++++
 src/runner.rs                                 | 14 ++++--
 tests/cucumber.rs                             |  3 ++
 tests/features/ninja.feature                  |  2 +-
 tests/steps/ir_steps.rs                       |  1 +
 tests/steps/ninja_steps.rs                    | 13 ++++++
 9 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md
index b9206e11..bbfad6ae 100644
--- a/docs/snapshot-testing-in-netsuke-using-insta.md
+++ b/docs/snapshot-testing-in-netsuke-using-insta.md
@@ -176,6 +176,7 @@ fn simple_manifest_ninja_snapshot() {
     let build_graph = BuildGraph::from_manifest(&manifest).expect("IR generation succeeded");
 
     // Generate Ninja file content from the IR
+    // The function returns Result
     let ninja_file = ninja_gen::generate(&build_graph)
         .expect("Ninja file generation succeeded");
 
@@ -196,9 +197,10 @@ Key points for Ninja snapshot tests:
   constructed directly for tests, but using the manifest→IR pipeline ensures
   realistic coverage.
 
-- Call the Ninja generation function (e.g. `ninja_gen::generate`), which
-  produces the Ninja file contents as a `String`. This function traverses the
-  IR and outputs rules and build statements in Ninja syntax.
+- Call the Ninja generation function (`ninja_gen::generate`), which
+  produces the Ninja file contents as a `Result`. This function traverses the
+  IR and outputs rules and build statements in Ninja syntax, returning an error
+  if any build edge references an undefined action.
 
 - As with IR, **determinism is crucial**. The Ninja output should list rules,
   targets, and dependencies in a consistent order. For example, if the IR does
diff --git a/src/diagnostics.rs b/src/diagnostics.rs
index dec61ffa..36d04fd5 100644
--- a/src/diagnostics.rs
+++ b/src/diagnostics.rs
@@ -29,7 +29,7 @@ use std::fmt::Display;
 ///     File::open(path).diag("open file")
 /// }
 /// ```
-#[allow(dead_code, reason = "unused after migration to anyhow")]
+#[expect(dead_code, reason = "unused after migration to anyhow")]
 pub(crate) trait ResultExt<T> {
     /// Attach a static context message to any error.
     ///
diff --git a/src/manifest.rs b/src/manifest.rs
index 484b583e..c7467a62 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -528,8 +528,9 @@ fn glob_paths(pattern: &str) -> std::result::Result<Vec<String>, Error> {
 ///
 /// Returns an error if YAML parsing or Jinja evaluation fails.
 fn from_str_named(yaml: &str, name: &str) -> Result<NetsukeManifest> {
-    let mut doc: YamlValue =
-        serde_yml::from_str(yaml).map_err(|e| map_yaml_error(e, yaml, name))?;
+    let mut doc: YamlValue = serde_yml::from_str(yaml).map_err(|e| ManifestError::Parse {
+        source: map_yaml_error(e, yaml, name),
+    })?;
 
     let mut jinja = Environment::new();
     jinja.set_undefined_behavior(UndefinedBehavior::Strict);
diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index 99905ca8..e2ed7615 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -38,6 +38,27 @@ macro_rules! write_flag {
 
 /// Generate a Ninja build file as a string.
 ///
+/// # Examples
+/// ```
+/// use crate::ast::Recipe;
+/// use crate::ir::{Action, BuildEdge, BuildGraph};
+/// use std::path::PathBuf;
+/// let mut graph = BuildGraph::default();
+/// graph.actions.insert("a".into(), Action {
+///     recipe: Recipe::Command { command: "true".into() },
+///     description: None, depfile: None, deps_format: None,
+///     pool: None, restat: false
+/// });
+/// graph.targets.insert(PathBuf::from("out"), BuildEdge {
+///     action_id: "a".into(), inputs: Vec::new(),
+///     explicit_outputs: vec![PathBuf::from("out")],
+///     implicit_outputs: Vec::new(), order_only_deps: Vec::new(),
+///     phony: false, always: false
+/// });
+/// let text = crate::ninja_gen::generate(&graph).expect("generate ninja");
+/// assert!(text.contains("rule a"));
+/// ```
+///
 /// # Errors
 ///
 /// Returns [`NinjaGenError`] if a build edge references an unknown action or
@@ -50,6 +71,28 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
 
 /// Write a Ninja build file to the provided writer.
 ///
+/// # Examples
+/// ```
+/// use crate::ast::Recipe;
+/// use crate::ir::{Action, BuildEdge, BuildGraph};
+/// use std::path::PathBuf;
+/// let mut graph = BuildGraph::default();
+/// graph.actions.insert("a".into(), Action {
+///     recipe: Recipe::Command { command: "true".into() },
+///     description: None, depfile: None, deps_format: None,
+///     pool: None, restat: false
+/// });
+/// graph.targets.insert(PathBuf::from("out"), BuildEdge {
+///     action_id: "a".into(), inputs: Vec::new(),
+///     explicit_outputs: vec![PathBuf::from("out")],
+///     implicit_outputs: Vec::new(), order_only_deps: Vec::new(),
+///     phony: false, always: false
+/// });
+/// let mut out = String::new();
+/// crate::ninja_gen::generate_into(&graph, &mut out).expect("format ninja");
+/// assert!(out.contains("build out: a"));
+/// ```
+///
 /// # Errors
 ///
 /// Returns [`NinjaGenError`] if a build edge references an unknown action or writing to the output fails.
diff --git a/src/runner.rs b/src/runner.rs
index c0ea5dc9..187cb14c 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -138,7 +138,13 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> {
     }
 
     let program = resolve_ninja_program();
-    run_ninja(program.as_path(), cli, build_path.as_ref(), &targets).context("run ninja")?;
+    run_ninja(program.as_path(), cli, build_path.as_ref(), &targets).with_context(|| {
+        format!(
+            "running {} with build file {}",
+            program.display(),
+            build_path.display()
+        )
+    })?;
     drop(tmp_file);
     Ok(())
 }
@@ -213,8 +219,10 @@ fn generate_ninja(cli: &Cli) -> Result<NinjaContent> {
     let manifest_path = resolve_manifest_path(cli);
     let manifest = manifest::from_path(&manifest_path)
         .with_context(|| format!("loading manifest at {}", manifest_path.display()))?;
-    let ast_json = serde_json::to_string_pretty(&manifest).context("serialising manifest")?;
-    debug!("AST:\n{ast_json}");
+    if tracing::enabled!(tracing::Level::DEBUG) {
+        let ast_json = serde_json::to_string_pretty(&manifest).context("serialising manifest")?;
+        debug!("AST:\n{ast_json}");
+    }
     let graph = BuildGraph::from_manifest(&manifest).context("building graph")?;
     let ninja = ninja_gen::generate(&graph).context("generating Ninja file")?;
     Ok(NinjaContent::new(ninja))
diff --git a/tests/cucumber.rs b/tests/cucumber.rs
index 7b9da4fb..5267767d 100644
--- a/tests/cucumber.rs
+++ b/tests/cucumber.rs
@@ -14,7 +14,10 @@ pub struct CliWorld {
     pub build_graph: Option<netsuke::ir::BuildGraph>,
     /// Generated Ninja file content.
     pub ninja: Option<String>,
+    /// Error message from Ninja generation.
     pub ninja_error: Option<String>,
+    /// Identifier of the action removed for negative tests.
+    pub removed_action_id: Option<String>,
     /// Status of the last process execution (true for success, false for
     /// failure).
     pub run_status: Option<bool>,
diff --git a/tests/features/ninja.feature b/tests/features/ninja.feature
index ee7df53f..d714fd90 100644
--- a/tests/features/ninja.feature
+++ b/tests/features/ninja.feature
@@ -26,5 +26,5 @@ Feature: Ninja file generation
     When the manifest file "tests/data/rules.yml" is compiled to IR
     And an action is removed from the graph
     And the ninja file is generated
-    Then ninja generation fails with "referenced by build edge was not found"
+    Then ninja generation fails mentioning the removed action id
 
diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs
index 66459fc5..b22d7d52 100644
--- a/tests/steps/ir_steps.rs
+++ b/tests/steps/ir_steps.rs
@@ -93,5 +93,6 @@ fn remove_action(world: &mut CliWorld) {
     let graph = world.build_graph.as_mut().expect("graph");
     if let Some(id) = graph.actions.keys().next().cloned() {
         graph.actions.remove(&id);
+        world.removed_action_id = Some(id);
     }
 }
diff --git a/tests/steps/ninja_steps.rs b/tests/steps/ninja_steps.rs
index 69e5f623..44c36c13 100644
--- a/tests/steps/ninja_steps.rs
+++ b/tests/steps/ninja_steps.rs
@@ -80,3 +80,16 @@ fn ninja_generation_fails(world: &mut CliWorld, text: String) {
         .expect("ninja error should be available");
     assert!(err.contains(&text));
 }
+
+#[then("ninja generation fails mentioning the removed action id")]
+fn ninja_generation_fails_with_removed_action_id(world: &mut CliWorld) {
+    let err = world
+        .ninja_error
+        .as_ref()
+        .expect("ninja error should be available");
+    let id = world
+        .removed_action_id
+        .as_ref()
+        .expect("removed action id should be available");
+    assert!(err.contains(id));
+}

From b76f11ae7f469455b087d57afb38e48be0ad545e Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Fri, 12 Sep 2025 14:03:28 +0100
Subject: [PATCH 07/12] Refine diagnostics and deterministic action removal

---
 src/diagnostics.rs      | 5 ++++-
 tests/steps/ir_steps.rs | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/diagnostics.rs b/src/diagnostics.rs
index 36d04fd5..d0ce0178 100644
--- a/src/diagnostics.rs
+++ b/src/diagnostics.rs
@@ -29,7 +29,10 @@ use std::fmt::Display;
 ///     File::open(path).diag("open file")
 /// }
 /// ```
-#[expect(dead_code, reason = "unused after migration to anyhow")]
+#[expect(
+    dead_code,
+    reason = "temporarily retained during anyhow migration; remove when no call sites remain"
+)]
 pub(crate) trait ResultExt<T> {
     /// Attach a static context message to any error.
     ///
diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs
index b22d7d52..297f69bc 100644
--- a/tests/steps/ir_steps.rs
+++ b/tests/steps/ir_steps.rs
@@ -91,7 +91,8 @@ fn ir_generation_fails(world: &mut CliWorld) {
 #[when("an action is removed from the graph")]
 fn remove_action(world: &mut CliWorld) {
     let graph = world.build_graph.as_mut().expect("graph");
-    if let Some(id) = graph.actions.keys().next().cloned() {
+    let first_edge_action = graph.targets.values().next().map(|e| e.action_id.clone());
+    if let Some(id) = first_edge_action {
         graph.actions.remove(&id);
         world.removed_action_id = Some(id);
     }

From 76bda1fe48b352129a64b4978528437264d14e1e Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sat, 13 Sep 2025 15:06:36 +0100
Subject: [PATCH 08/12] Use crate paths in Ninja generator docs

---
 src/ninja_gen.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index e2ed7615..1ea13626 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -40,8 +40,8 @@ macro_rules! write_flag {
 ///
 /// # Examples
 /// ```
-/// use crate::ast::Recipe;
-/// use crate::ir::{Action, BuildEdge, BuildGraph};
+/// use netsuke::ast::Recipe;
+/// use netsuke::ir::{Action, BuildEdge, BuildGraph};
 /// use std::path::PathBuf;
 /// let mut graph = BuildGraph::default();
 /// graph.actions.insert("a".into(), Action {
@@ -55,7 +55,7 @@ macro_rules! write_flag {
 ///     implicit_outputs: Vec::new(), order_only_deps: Vec::new(),
 ///     phony: false, always: false
 /// });
-/// let text = crate::ninja_gen::generate(&graph).expect("generate ninja");
+/// let text = netsuke::ninja_gen::generate(&graph).expect("generate ninja");
 /// assert!(text.contains("rule a"));
 /// ```
 ///
@@ -73,8 +73,8 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
 ///
 /// # Examples
 /// ```
-/// use crate::ast::Recipe;
-/// use crate::ir::{Action, BuildEdge, BuildGraph};
+/// use netsuke::ast::Recipe;
+/// use netsuke::ir::{Action, BuildEdge, BuildGraph};
 /// use std::path::PathBuf;
 /// let mut graph = BuildGraph::default();
 /// graph.actions.insert("a".into(), Action {
@@ -89,7 +89,7 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
 ///     phony: false, always: false
 /// });
 /// let mut out = String::new();
-/// crate::ninja_gen::generate_into(&graph, &mut out).expect("format ninja");
+/// netsuke::ninja_gen::generate_into(&graph, &mut out).expect("format ninja");
 /// assert!(out.contains("build out: a"));
 /// ```
 ///

From 90998921b0d5ebeede20251966d8bef808d12a52 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sat, 13 Sep 2025 15:07:01 +0100
Subject: [PATCH 09/12] Clarify generate return type in snapshot guide

---
 docs/snapshot-testing-in-netsuke-using-insta.md |  8 ++++----
 src/ninja_gen.rs                                | 12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md
index bbfad6ae..d440861a 100644
--- a/docs/snapshot-testing-in-netsuke-using-insta.md
+++ b/docs/snapshot-testing-in-netsuke-using-insta.md
@@ -176,7 +176,7 @@ fn simple_manifest_ninja_snapshot() {
     let build_graph = BuildGraph::from_manifest(&manifest).expect("IR generation succeeded");
 
     // Generate Ninja file content from the IR
-    // The function returns Result
+    // The function returns `Result<String, NinjaGenError>`
     let ninja_file = ninja_gen::generate(&build_graph)
         .expect("Ninja file generation succeeded");
 
@@ -198,9 +198,9 @@ Key points for Ninja snapshot tests:
   realistic coverage.
 
 - Call the Ninja generation function (`ninja_gen::generate`), which
-  produces the Ninja file contents as a `Result`. This function traverses the
-  IR and outputs rules and build statements in Ninja syntax, returning an error
-  if any build edge references an undefined action.
+  yields a `Result<String, NinjaGenError>`. This function traverses the IR and
+  outputs rules and build statements in Ninja syntax, returning an error if any
+  build edge references an undefined action.
 
 - As with IR, **determinism is crucial**. The Ninja output should list rules,
   targets, and dependencies in a consistent order. For example, if the IR does
diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index 1ea13626..e2ed7615 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -40,8 +40,8 @@ macro_rules! write_flag {
 ///
 /// # Examples
 /// ```
-/// use netsuke::ast::Recipe;
-/// use netsuke::ir::{Action, BuildEdge, BuildGraph};
+/// use crate::ast::Recipe;
+/// use crate::ir::{Action, BuildEdge, BuildGraph};
 /// use std::path::PathBuf;
 /// let mut graph = BuildGraph::default();
 /// graph.actions.insert("a".into(), Action {
@@ -55,7 +55,7 @@ macro_rules! write_flag {
 ///     implicit_outputs: Vec::new(), order_only_deps: Vec::new(),
 ///     phony: false, always: false
 /// });
-/// let text = netsuke::ninja_gen::generate(&graph).expect("generate ninja");
+/// let text = crate::ninja_gen::generate(&graph).expect("generate ninja");
 /// assert!(text.contains("rule a"));
 /// ```
 ///
@@ -73,8 +73,8 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
 ///
 /// # Examples
 /// ```
-/// use netsuke::ast::Recipe;
-/// use netsuke::ir::{Action, BuildEdge, BuildGraph};
+/// use crate::ast::Recipe;
+/// use crate::ir::{Action, BuildEdge, BuildGraph};
 /// use std::path::PathBuf;
 /// let mut graph = BuildGraph::default();
 /// graph.actions.insert("a".into(), Action {
@@ -89,7 +89,7 @@ pub fn generate(graph: &BuildGraph) -> Result<String, NinjaGenError> {
 ///     phony: false, always: false
 /// });
 /// let mut out = String::new();
-/// netsuke::ninja_gen::generate_into(&graph, &mut out).expect("format ninja");
+/// crate::ninja_gen::generate_into(&graph, &mut out).expect("format ninja");
 /// assert!(out.contains("build out: a"));
 /// ```
 ///

From bfcbb4bebb33d2d4247d7067907c394db030205c Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sat, 13 Sep 2025 15:22:23 +0100
Subject: [PATCH 10/12] Handle Ninja generation errors in snapshot docs

---
 docs/snapshot-testing-in-netsuke-using-insta.md | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md
index d440861a..d120466f 100644
--- a/docs/snapshot-testing-in-netsuke-using-insta.md
+++ b/docs/snapshot-testing-in-netsuke-using-insta.md
@@ -176,9 +176,11 @@ fn simple_manifest_ninja_snapshot() {
     let build_graph = BuildGraph::from_manifest(&manifest).expect("IR generation succeeded");
 
     // Generate Ninja file content from the IR
-    // The function returns `Result<String, NinjaGenError>`
-    let ninja_file = ninja_gen::generate(&build_graph)
-        .expect("Ninja file generation succeeded");
+    // `generate` returns `Result<String, NinjaGenError>`; handle errors
+    let ninja_file = match ninja_gen::generate(&build_graph) {
+        Ok(ninja) => ninja,
+        Err(e) => panic!("Ninja file generation failed: {e}"),
+    };
 
     // The output is a multi-line Ninja build script (as a String)
     // Ensure the output is deterministic
@@ -191,6 +193,10 @@ fn simple_manifest_ninja_snapshot() {
 }
 ```
 
+The match explicitly handles the `Result` from `generate` so any formatting or
+missing action errors surface during tests. Production code should propagate
+the error and report it with `miette` rather than panicking.
+
 Key points for Ninja snapshot tests:
 
 - Use a known manifest input and first derive the IR. An IR can also be

From 4e51ea2a5cc77b3d1a4db13df195fcd1a995ba8e Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sun, 14 Sep 2025 00:40:02 +0100
Subject: [PATCH 11/12] Swap allows for expects in Ninja steps

---
 tests/steps/ir_steps.rs    |  4 ++--
 tests/steps/ninja_steps.rs | 16 ++++++----------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs
index 297f69bc..7dcf6425 100644
--- a/tests/steps/ir_steps.rs
+++ b/tests/steps/ir_steps.rs
@@ -91,8 +91,8 @@ fn ir_generation_fails(world: &mut CliWorld) {
 #[when("an action is removed from the graph")]
 fn remove_action(world: &mut CliWorld) {
     let graph = world.build_graph.as_mut().expect("graph");
-    let first_edge_action = graph.targets.values().next().map(|e| e.action_id.clone());
-    if let Some(id) = first_edge_action {
+    let first_action = graph.targets.values().next().map(|e| e.action_id.clone());
+    if let Some(id) = first_action {
         graph.actions.remove(&id);
         world.removed_action_id = Some(id);
     }
diff --git a/tests/steps/ninja_steps.rs b/tests/steps/ninja_steps.rs
index 44c36c13..412af163 100644
--- a/tests/steps/ninja_steps.rs
+++ b/tests/steps/ninja_steps.rs
@@ -22,11 +22,11 @@ fn generate_ninja(world: &mut CliWorld) {
     }
 }
 
-#[allow(
+#[then(expr = "the ninja file contains {string}")]
+#[expect(
     clippy::needless_pass_by_value,
     reason = "Cucumber requires owned String arguments"
 )]
-#[then(expr = "the ninja file contains {string}")]
 fn ninja_contains(world: &mut CliWorld, text: String) {
     let ninja = world
         .ninja
@@ -35,11 +35,11 @@ fn ninja_contains(world: &mut CliWorld, text: String) {
     assert!(ninja.contains(&text));
 }
 
-#[allow(
+#[then(expr = "shlex splitting command {int} yields {string}")]
+#[expect(
     clippy::needless_pass_by_value,
     reason = "Cucumber requires owned String arguments"
 )]
-#[then(expr = "shlex splitting command {int} yields {string}")]
 fn ninja_command_tokens(world: &mut CliWorld, index: usize, expected: String) {
     let ninja = world
         .ninja
@@ -59,20 +59,16 @@ fn ninja_command_tokens(world: &mut CliWorld, index: usize, expected: String) {
     assert_eq!(words, expected);
 }
 
-#[allow(
-    clippy::needless_pass_by_value,
-    reason = "Cucumber requires owned String arguments"
-)]
 #[then(expr = "shlex splitting the command yields {string}")]
 fn ninja_first_command_tokens(world: &mut CliWorld, expected: String) {
     ninja_command_tokens(world, 2, expected);
 }
 
-#[allow(
+#[then(expr = "ninja generation fails with {string}")]
+#[expect(
     clippy::needless_pass_by_value,
     reason = "Cucumber requires owned String arguments"
 )]
-#[then(expr = "ninja generation fails with {string}")]
 fn ninja_generation_fails(world: &mut CliWorld, text: String) {
     let err = world
         .ninja_error

From 8c4c456f4789dddaa182401467ab81680553d819 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sun, 14 Sep 2025 11:53:16 +0100
Subject: [PATCH 12/12] Box YAML diagnostics in manifest errors

---
 docs/netsuke-design.md |  8 ++++----
 src/manifest.rs        | 12 ++++++++----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index a4c4ed91..48dedec1 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1370,14 +1370,14 @@ pub enum ManifestError {
     Parse {
         #[source]
         #[diagnostic_source]
-        source: YamlDiagnostic,
+        source: Box<dyn Diagnostic + Send + Sync + 'static>,
     },
 }
 ```
 
-`ManifestError::Parse` preserves the rich diagnostic, allowing `miette` to show
-the offending YAML snippet. All new user-facing errors with source context must
-follow this model.
+`ManifestError::Parse` boxes the diagnostic to preserve the rich error so
+`miette` can show the offending YAML snippet. All new user-facing errors with
+source context must follow this model.
 
 Common use cases requiring `miette` diagnostics include:
 
diff --git a/src/manifest.rs b/src/manifest.rs
index c7467a62..bc2ba5cf 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -84,11 +84,15 @@ pub enum ManifestError {
     Parse {
         #[source]
         #[diagnostic_source]
-        source: YamlDiagnostic,
+        source: Box<dyn Diagnostic + Send + Sync + 'static>,
     },
 }
 
-fn map_yaml_error(err: YamlError, src: &str, name: &str) -> YamlDiagnostic {
+fn map_yaml_error(
+    err: YamlError,
+    src: &str,
+    name: &str,
+) -> Box<dyn Diagnostic + Send + Sync + 'static> {
     let loc = err.location();
     let (line, col, span) = loc.map_or((1, 1, None), |l| {
         (l.line(), l.column(), Some(to_span(src, l)))
@@ -101,13 +105,13 @@ fn map_yaml_error(err: YamlError, src: &str, name: &str) -> YamlDiagnostic {
         message.push_str(h);
     }
 
-    YamlDiagnostic {
+    Box::new(YamlDiagnostic {
         src: NamedSource::new(name, src.to_string()),
         span,
         help: hint,
         source: err,
         message,
-    }
+    })
 }
 
 /// Resolve the value of an environment variable for the `env()` Jinja helper.