From 5e76f59b2729784040e3579ad59b198922eba285 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Sat, 9 Aug 2025 00:27:18 +0100
Subject: [PATCH 1/3] Add fixtures for control flow tests

---
 docs/netsuke-design.md           | 33 ++++++++-------
 docs/roadmap.md                  |  6 +--
 src/ast.rs                       |  4 +-
 src/manifest.rs                  | 27 +++++--------
 tests/data/jinja_for.yml         | 10 +++++
 tests/data/jinja_for_invalid.yml |  8 ++++
 tests/data/jinja_if.yml          |  6 +++
 tests/features/manifest.feature  | 17 ++++++++
 tests/manifest_jinja_tests.rs    | 69 ++++++++++++++++++++++++++++++++
 tests/steps/manifest_steps.rs    | 33 +++++++++++++++
 10 files changed, 177 insertions(+), 36 deletions(-)
 create mode 100644 tests/data/jinja_for.yml
 create mode 100644 tests/data/jinja_for_invalid.yml
 create mode 100644 tests/data/jinja_if.yml

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index 8c0fb969..77ee6add 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -161,9 +161,9 @@ level keys.
   future evolution of the schema while maintaining backward compatibility. This
   version string should be parsed and validated using the `semver` crate.[^4]
 
-- `vars`: A mapping of global key-value string pairs. These variables are
-  available for substitution in rule commands and target definitions and are
-  exposed to the Jinja templating context.
+- `vars`: A mapping of global key-value pairs. Values may be strings, numbers,
+  booleans, or sequences. These variables seed the Jinja templating context and
+  drive control flow within the manifest.
 
 - `macros`: An optional list of Jinja macro definitions. Each item provides a
   `signature` string using standard Jinja syntax and a `body` declared with the
@@ -414,7 +414,7 @@ pub struct NetsukeManifest {
     pub netsuke_version: Version,
 
     #[serde(default)]
-    pub vars: HashMap<String, String>,
+    pub vars: HashMap<String, serde_yml::Value>,
 
     #[serde(default)]
     pub rules: Vec<Rule>,
@@ -466,7 +466,7 @@ pub struct Target {
     pub order_only_deps: StringOrList,
 
     #[serde(default)]
-    pub vars: HashMap<String, String>,
+    pub vars: HashMap<String, serde_yml::Value>,
 
     /// Run this target when requested even if a file with the same name exists.
     #[serde(default)]
@@ -585,15 +585,14 @@ Unknown fields are rejected to surface user errors early. `StringOrList`
 provides a default `Empty` variant, so optional lists are trivial to represent.
 The manifest version is parsed using the `semver` crate to validate that it
 follows semantic versioning rules. Global and target variable maps now share
-the `HashMap<String, String>` type for consistency. This keeps YAML manifests
-concise while ensuring forward compatibility. Targets also accept optional
-`phony` and `always` booleans. They default to `false`, making it explicit when
-an action should run regardless of file timestamps. Targets listed in the
-`actions` section are deserialised using a custom helper so they are always
-treated as `phony` tasks. This ensures preparation actions never generate build
-artefacts. Convenience functions in `src/manifest.rs` load a manifest from a
-string or a file path, returning `anyhow::Result` for straightforward error
-handling.
+the `HashMap<String, serde_yml::Value>` type so booleans and sequences are
+preserved for Jinja control flow. Targets also accept optional `phony` and
+`always` booleans. They default to `false`, making it explicit when an action
+should run regardless of file timestamps. Targets listed in the `actions`
+section are deserialised using a custom helper so they are always treated as
+`phony` tasks. This ensures preparation actions never generate build artefacts.
+Convenience functions in `src/manifest.rs` load a manifest from a string or a
+file path, returning `anyhow::Result` for straightforward error handling.
 
 ### 3.5 Testing
 
@@ -666,6 +665,12 @@ lenient undefined behaviour. The resulting YAML is parsed to obtain the global
 variables, which are then injected into the environment before a second, strict
 render pass produces the final manifest for deserialisation.
 
+The parser copies `vars` values into the environment using
+`Value::from_serializable`. This preserves native YAML types so Jinja's
+`{% if %}` and `{% for %}` constructs can branch on booleans or iterate over
+sequences. Attempting to iterate over a non-sequence results in a render error
+surfaced during manifest loading.
+
 ### 4.3 User-Defined Macros
 
 Netsuke allows users to declare reusable Jinja macros directly in the manifest.
diff --git a/docs/roadmap.md b/docs/roadmap.md
index a2e32ab5..d1354ad2 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -88,9 +88,9 @@ configurations with variables, control flow, and custom functions.
 
 - [ ] **Dynamic Features and Custom Functions:**
 
-  - [ ] Implement support for basic Jinja control structures (`{% if %}` and
-        `{% for %}`) 
-    
+  - [x] Implement support for basic Jinja control structures (`{% if %}` and
+        `{% for %}`)
+
   - [ ] Implement the foreach key for target generation.
 
   - [ ] Implement the essential custom Jinja function env(var_name) to read
diff --git a/src/ast.rs b/src/ast.rs
index fe85b0e9..2ff27350 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -61,7 +61,7 @@ pub struct NetsukeManifest {
 
     /// Global key/value pairs available to recipes.
     #[serde(default)]
-    pub vars: HashMap<String, String>,
+    pub vars: HashMap<String, serde_yml::Value>,
 
     /// Named rule templates that can be referenced by targets.
     #[serde(default)]
@@ -187,7 +187,7 @@ pub struct Target {
 
     /// Target-scoped variables available during command execution.
     #[serde(default)]
-    pub vars: HashMap<String, String>,
+    pub vars: HashMap<String, serde_yml::Value>,
 
     /// Declares that the target does not correspond to a real file.
     #[serde(default)]
diff --git a/src/manifest.rs b/src/manifest.rs
index 23edb6ca..2e612973 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -7,7 +7,7 @@
 use crate::ast::NetsukeManifest;
 use anyhow::{Context, Result};
 use minijinja::{Environment, UndefinedBehavior, context, value::Value};
-use std::{collections::HashMap, fs, path::Path};
+use std::{fs, path::Path};
 
 /// Parse a manifest string using Jinja for templating.
 ///
@@ -48,22 +48,15 @@ pub fn from_str(yaml: &str) -> Result<NetsukeManifest> {
 
     let doc: serde_yml::Value =
         serde_yml::from_str(&rendered).context("first-pass YAML parse error")?;
-    let vars = doc
-        .get("vars")
-        .and_then(|v| v.as_mapping())
-        .map(|m| {
-            m.iter()
-                .filter_map(|(k, v)| k.as_str().and_then(|key| v.as_str().map(|val| (key, val))))
-                .map(|(k, v)| (k.to_string(), v.to_string()))
-                .collect::<HashMap<_, _>>()
-        })
-        .unwrap_or_default();
-
-    // Populate the environment with the extracted variables for subsequent
-    // rendering. Undefined variables now trigger errors to surface template
-    // mistakes early.
-    for (key, value) in vars {
-        env.add_global(key, Value::from(value));
+    if let Some(vars) = doc.get("vars").and_then(|v| v.as_mapping()) {
+        // Copy each key-value pair into the environment, preserving native YAML
+        // types so control structures like `{% if %}` and `{% for %}` can operate
+        // on booleans and sequences.
+        for (k, v) in vars {
+            if let Some(key) = k.as_str() {
+                env.add_global(key, Value::from_serialize(v.clone()));
+            }
+        }
     }
 
     env.set_undefined_behavior(UndefinedBehavior::Strict);
diff --git a/tests/data/jinja_for.yml b/tests/data/jinja_for.yml
new file mode 100644
index 00000000..f93a84b0
--- /dev/null
+++ b/tests/data/jinja_for.yml
@@ -0,0 +1,10 @@
+netsuke_version: 1.0.0
+vars:
+  items:
+    - foo
+    - bar
+targets:
+{% for item in items %}
+  - name: "{{ item }}"
+    command: "echo {{ item }}"
+{% endfor %}
diff --git a/tests/data/jinja_for_invalid.yml b/tests/data/jinja_for_invalid.yml
new file mode 100644
index 00000000..2ecb50df
--- /dev/null
+++ b/tests/data/jinja_for_invalid.yml
@@ -0,0 +1,8 @@
+netsuke_version: 1.0.0
+vars:
+  items: 1
+targets:
+{% for item in items %}
+  - name: "{{ item }}"
+    command: "echo {{ item }}"
+{% endfor %}
diff --git a/tests/data/jinja_if.yml b/tests/data/jinja_if.yml
new file mode 100644
index 00000000..416880b3
--- /dev/null
+++ b/tests/data/jinja_if.yml
@@ -0,0 +1,6 @@
+netsuke_version: 1.0.0
+vars:
+  enable: true
+targets:
+  - name: hello
+    command: "{% if enable %}echo on{% else %}echo off{% endif %}"
diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature
index 042212a1..ada6218a 100644
--- a/tests/features/manifest.feature
+++ b/tests/features/manifest.feature
@@ -55,3 +55,20 @@ Feature: Manifest Parsing
     Given the manifest file "tests/data/jinja_undefined.yml" is parsed
     When the parsing result is checked
     Then parsing the manifest fails
+
+  Scenario: Rendering Jinja conditionals in a manifest
+    Given the manifest file "tests/data/jinja_if.yml" is parsed
+    When the manifest is checked
+    Then the first target command is "echo on"
+
+  Scenario: Rendering Jinja loops in a manifest
+    Given the manifest file "tests/data/jinja_for.yml" is parsed
+    When the manifest is checked
+    Then the manifest has 2 targets
+    And the target 1 command is "echo foo"
+    And the target 2 command is "echo bar"
+
+  Scenario: Parsing fails when a Jinja loop iterates over a non-list
+    Given the manifest file "tests/data/jinja_for_invalid.yml" is parsed
+    When the parsing result is checked
+    Then parsing the manifest fails
diff --git a/tests/manifest_jinja_tests.rs b/tests/manifest_jinja_tests.rs
index 245b05f2..10391199 100644
--- a/tests/manifest_jinja_tests.rs
+++ b/tests/manifest_jinja_tests.rs
@@ -46,3 +46,72 @@ targets:
 
     assert!(manifest::from_str(yaml).is_err());
 }
+
+#[rstest]
+#[case(true, "echo on")]
+#[case(false, "echo off")]
+fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) {
+    let yaml = format!(
+        concat!(
+            "netsuke_version: 1.0.0\n",
+            "vars:\n",
+            "  flag: {flag}\n",
+            "targets:\n",
+            "  - name: test\n",
+            "    command: {{% if flag %}}echo on{{% else %}}echo off{{% endif %}}\n"
+        ),
+        flag = flag
+    );
+
+    let manifest = manifest::from_str(&yaml).expect("parse");
+    let first = manifest.targets.first().expect("target");
+    if let Recipe::Command { command } = &first.recipe {
+        assert_eq!(command, expected);
+    } else {
+        panic!("Expected command recipe, got: {:?}", first.recipe);
+    }
+}
+
+#[rstest]
+fn renders_for_loops() {
+    let yaml = r#"
+netsuke_version: 1.0.0
+vars:
+  items:
+    - a
+    - b
+targets:
+{% for item in items %}
+  - name: "{{ item }}"
+    command: "echo {{ item }}"
+{% endfor %}
+"#;
+
+    let manifest = manifest::from_str(yaml).expect("parse");
+    assert_eq!(manifest.targets.len(), 2);
+    let names: Vec<_> = manifest
+        .targets
+        .iter()
+        .map(|t| match &t.name {
+            netsuke::ast::StringOrList::String(s) => s.clone(),
+            other => panic!("Expected String, got: {other:?}"),
+        })
+        .collect();
+    assert_eq!(names, vec!["a", "b"]);
+}
+
+#[rstest]
+fn for_loop_non_iterable_errors() {
+    let yaml = r#"
+netsuke_version: 1.0.0
+vars:
+  items: 1
+targets:
+{% for item in items %}
+  - name: "{{ item }}"
+    command: "echo {{ item }}"
+{% endfor %}
+"#;
+
+    assert!(manifest::from_str(yaml).is_err());
+}
diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs
index af99bcb0..f860f734 100644
--- a/tests/steps/manifest_steps.rs
+++ b/tests/steps/manifest_steps.rs
@@ -116,3 +116,36 @@ fn first_target_command(world: &mut CliWorld, command: String) {
         panic!("Expected command recipe, got: {:?}", first.recipe);
     }
 }
+
+#[then(expr = "the manifest has {int} targets")]
+fn manifest_has_targets(world: &mut CliWorld, count: usize) {
+    let manifest = world.manifest.as_ref().expect("manifest");
+    assert_eq!(manifest.targets.len(), count);
+}
+
+#[then(expr = "the target {int} name is {string}")]
+fn target_name_n(world: &mut CliWorld, index: usize, name: String) {
+    let manifest = world.manifest.as_ref().expect("manifest");
+    let target = manifest
+        .targets
+        .get(index - 1)
+        .unwrap_or_else(|| panic!("missing target {index}"));
+    match &target.name {
+        StringOrList::String(value) => assert_eq!(value, &name),
+        other => panic!("Expected StringOrList::String, got: {other:?}"),
+    }
+}
+
+#[then(expr = "the target {int} command is {string}")]
+fn target_command_n(world: &mut CliWorld, index: usize, command: String) {
+    let manifest = world.manifest.as_ref().expect("manifest");
+    let target = manifest
+        .targets
+        .get(index - 1)
+        .unwrap_or_else(|| panic!("missing target {index}"));
+    if let Recipe::Command { command: actual } = &target.recipe {
+        assert_eq!(actual, &command);
+    } else {
+        panic!("Expected command recipe, got: {:?}", target.recipe);
+    }
+}

From b5c61a56427647a59e23c144bc568177eae8733d Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Mon, 11 Aug 2025 10:35:50 +0100
Subject: [PATCH 2/3] Enforce string keys in vars and expand Jinja tests

---
 docs/netsuke-design.md           |   9 +--
 src/ast.rs                       |   2 +-
 src/manifest.rs                  |   9 +--
 tests/data/jinja_for.yml         |   1 +
 tests/data/jinja_for_invalid.yml |   1 +
 tests/features/manifest.feature  |   5 +-
 tests/manifest_jinja_tests.rs    | 112 +++++++++++++++----------------
 tests/steps/manifest_steps.rs    |  61 +++++++++++------
 8 files changed, 114 insertions(+), 86 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index 77ee6add..381d6bca 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -161,9 +161,9 @@ level keys.
   future evolution of the schema while maintaining backward compatibility. This
   version string should be parsed and validated using the `semver` crate.[^4]
 
-- `vars`: A mapping of global key-value pairs. Values may be strings, numbers,
-  booleans, or sequences. These variables seed the Jinja templating context and
-  drive control flow within the manifest.
+- `vars`: A mapping of global key-value pairs. Keys must be strings. Values may
+  be strings, numbers, booleans, or sequences. These variables seed the Jinja
+  templating context and drive control flow within the manifest.
 
 - `macros`: An optional list of Jinja macro definitions. Each item provides a
   `signature` string using standard Jinja syntax and a `body` declared with the
@@ -668,7 +668,8 @@ render pass produces the final manifest for deserialisation.
 The parser copies `vars` values into the environment using
 `Value::from_serializable`. This preserves native YAML types so Jinja's
 `{% if %}` and `{% for %}` constructs can branch on booleans or iterate over
-sequences. Attempting to iterate over a non-sequence results in a render error
+sequences. Keys must be strings; any non-string key causes manifest parsing to
+fail. Attempting to iterate over a non-sequence results in a render error
 surfaced during manifest loading.
 
 ### 4.3 User-Defined Macros
diff --git a/src/ast.rs b/src/ast.rs
index 2ff27350..2bf75673 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -19,6 +19,7 @@
 
 use semver::Version;
 use serde::{Deserialize, Serialize, de::Deserializer};
+use std::collections::HashMap;
 
 fn deserialize_actions<'de, D>(deserializer: D) -> Result<Vec<Target>, D::Error>
 where
@@ -30,7 +31,6 @@ where
     }
     Ok(actions)
 }
-use std::collections::HashMap;
 
 /// Top-level manifest structure parsed from a `Netsukefile`.
 ///
diff --git a/src/manifest.rs b/src/manifest.rs
index 2e612973..6df3b1d5 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -5,7 +5,7 @@
 //! They wrap `serde_yml` and add basic file handling.
 
 use crate::ast::NetsukeManifest;
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, bail};
 use minijinja::{Environment, UndefinedBehavior, context, value::Value};
 use std::{fs, path::Path};
 
@@ -53,9 +53,10 @@ pub fn from_str(yaml: &str) -> Result<NetsukeManifest> {
         // types so control structures like `{% if %}` and `{% for %}` can operate
         // on booleans and sequences.
         for (k, v) in vars {
-            if let Some(key) = k.as_str() {
-                env.add_global(key, Value::from_serialize(v.clone()));
-            }
+            let Some(key) = k.as_str() else {
+                bail!("non-string key in 'vars' mapping: {k:?}");
+            };
+            env.add_global(key, Value::from_serialize(v.clone()));
         }
     }
 
diff --git a/tests/data/jinja_for.yml b/tests/data/jinja_for.yml
index f93a84b0..ad4f213e 100644
--- a/tests/data/jinja_for.yml
+++ b/tests/data/jinja_for.yml
@@ -8,3 +8,4 @@ targets:
   - name: "{{ item }}"
     command: "echo {{ item }}"
 {% endfor %}
+
diff --git a/tests/data/jinja_for_invalid.yml b/tests/data/jinja_for_invalid.yml
index 2ecb50df..cc86f552 100644
--- a/tests/data/jinja_for_invalid.yml
+++ b/tests/data/jinja_for_invalid.yml
@@ -6,3 +6,4 @@ targets:
   - name: "{{ item }}"
     command: "echo {{ item }}"
 {% endfor %}
+
diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature
index ada6218a..e20d3b22 100644
--- a/tests/features/manifest.feature
+++ b/tests/features/manifest.feature
@@ -59,13 +59,16 @@ Feature: Manifest Parsing
   Scenario: Rendering Jinja conditionals in a manifest
     Given the manifest file "tests/data/jinja_if.yml" is parsed
     When the manifest is checked
-    Then the first target command is "echo on"
+    Then the first target name is "hello"
+    And the first target command is "echo on"
 
   Scenario: Rendering Jinja loops in a manifest
     Given the manifest file "tests/data/jinja_for.yml" is parsed
     When the manifest is checked
     Then the manifest has 2 targets
+    And the target 1 name is "foo"
     And the target 1 command is "echo foo"
+    And the target 2 name is "bar"
     And the target 2 command is "echo bar"
 
   Scenario: Parsing fails when a Jinja loop iterates over a non-list
diff --git a/tests/manifest_jinja_tests.rs b/tests/manifest_jinja_tests.rs
index 10391199..14a940ea 100644
--- a/tests/manifest_jinja_tests.rs
+++ b/tests/manifest_jinja_tests.rs
@@ -3,18 +3,17 @@
 use netsuke::{ast::Recipe, manifest};
 use rstest::rstest;
 
+fn manifest_yaml(body: &str) -> String {
+    format!("netsuke_version: 1.0.0\n{body}")
+}
+
 #[rstest]
 fn renders_global_vars() {
-    let yaml = r"
-netsuke_version: 1.0.0
-vars:
-  who: world
-targets:
-  - name: hello
-    command: echo {{ who }}
-";
-
-    let manifest = manifest::from_str(yaml).expect("parse");
+    let yaml = manifest_yaml(
+        "vars:\n  who: world\ntargets:\n  - name: hello\n    command: echo {{ who }}\n",
+    );
+
+    let manifest = manifest::from_str(&yaml).expect("parse");
     let first = manifest.targets.first().expect("target");
     if let Recipe::Command { command } = &first.recipe {
         assert_eq!(command, "echo world");
@@ -25,43 +24,32 @@ targets:
 
 #[rstest]
 fn undefined_variable_errors() {
-    let yaml = r"
-netsuke_version: 1.0.0
-targets:
-  - name: hello
-    command: echo {{ missing }}
-";
-
-    assert!(manifest::from_str(yaml).is_err());
+    let yaml = manifest_yaml("targets:\n  - name: hello\n    command: echo {{ missing }}\n");
+
+    assert!(manifest::from_str(&yaml).is_err());
 }
 
 #[rstest]
 fn syntax_error_errors() {
-    let yaml = r"
-netsuke_version: 1.0.0
-targets:
-  - name: hello
-    command: echo {{ who
-";
-
-    assert!(manifest::from_str(yaml).is_err());
+    let yaml = manifest_yaml("targets:\n  - name: hello\n    command: echo {{ who\n");
+
+    assert!(manifest::from_str(&yaml).is_err());
 }
 
 #[rstest]
 #[case(true, "echo on")]
 #[case(false, "echo off")]
 fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) {
-    let yaml = format!(
+    let yaml = manifest_yaml(&format!(
         concat!(
-            "netsuke_version: 1.0.0\n",
             "vars:\n",
             "  flag: {flag}\n",
             "targets:\n",
             "  - name: test\n",
-            "    command: {{% if flag %}}echo on{{% else %}}echo off{{% endif %}}\n"
+            "    command: {{% if flag %}}echo on{{% else %}}echo off{{% endif %}}\n",
         ),
         flag = flag
-    );
+    ));
 
     let manifest = manifest::from_str(&yaml).expect("parse");
     let first = manifest.targets.first().expect("target");
@@ -74,20 +62,11 @@ fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) {
 
 #[rstest]
 fn renders_for_loops() {
-    let yaml = r#"
-netsuke_version: 1.0.0
-vars:
-  items:
-    - a
-    - b
-targets:
-{% for item in items %}
-  - name: "{{ item }}"
-    command: "echo {{ item }}"
-{% endfor %}
-"#;
-
-    let manifest = manifest::from_str(yaml).expect("parse");
+    let yaml = manifest_yaml(
+        "vars:\n  items:\n    - a\n    - b\ntargets:\n{% for item in items %}\n  - name: \"{{ item }}\"\n    command: \"echo {{ item }}\"\n{% endfor %}\n",
+    );
+
+    let manifest = manifest::from_str(&yaml).expect("parse");
     assert_eq!(manifest.targets.len(), 2);
     let names: Vec<_> = manifest
         .targets
@@ -98,20 +77,41 @@ targets:
         })
         .collect();
     assert_eq!(names, vec!["a", "b"]);
+
+    let commands: Vec<_> = manifest
+        .targets
+        .iter()
+        .map(|t| match &t.recipe {
+            Recipe::Command { command } => command.clone(),
+            other => panic!("Expected command recipe, got: {other:?}"),
+        })
+        .collect();
+    assert_eq!(commands, vec!["echo a", "echo b"]);
 }
 
 #[rstest]
 fn for_loop_non_iterable_errors() {
-    let yaml = r#"
-netsuke_version: 1.0.0
-vars:
-  items: 1
-targets:
-{% for item in items %}
-  - name: "{{ item }}"
-    command: "echo {{ item }}"
-{% endfor %}
-"#;
-
-    assert!(manifest::from_str(yaml).is_err());
+    let yaml = manifest_yaml(
+        "vars:\n  items: 1\ntargets:\n{% for item in items %}\n  - name: \"{{ item }}\"\n    command: \"echo {{ item }}\"\n{% endfor %}\n",
+    );
+
+    assert!(manifest::from_str(&yaml).is_err());
+}
+
+#[rstest]
+fn undefined_in_if_errors() {
+    let yaml = manifest_yaml(
+        "targets:\n  - name: test\n    command: {% if missing %}echo hi{% endif %}\n",
+    );
+
+    assert!(manifest::from_str(&yaml).is_err());
+}
+
+#[rstest]
+fn undefined_in_for_errors() {
+    let yaml = manifest_yaml(
+        "targets:\n{% for item in missing %}\n  - name: \"{{ item }}\"\n    command: \"echo {{ item }}\"\n{% endfor %}\n",
+    );
+
+    assert!(manifest::from_str(&yaml).is_err());
 }
diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs
index f860f734..5b5c36c9 100644
--- a/tests/steps/manifest_steps.rs
+++ b/tests/steps/manifest_steps.rs
@@ -1,8 +1,4 @@
-//! Step definitions for manifest parsing scenarios.
-#![expect(
-    clippy::needless_pass_by_value,
-    reason = "Cucumber requires owned String arguments"
-)]
+//! Step definitions for manifest feature tests.
 
 use crate::CliWorld;
 use cucumber::{given, then, when};
@@ -27,28 +23,40 @@ fn parse_manifest_inner(world: &mut CliWorld, path: &str) {
 fn assert_manifest(world: &CliWorld) {
     assert!(
         world.manifest.is_some(),
-        "manifest should have been parsed successfully"
+        "manifest should have been parsed successfully",
     );
 }
 
 fn assert_parsed(world: &CliWorld) {
     assert!(
         world.manifest.is_some() || world.manifest_error.is_some(),
-        "manifest should have been parsed"
+        "manifest should have been parsed",
     );
 }
 
 #[given(expr = "the manifest file {string} is parsed")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn given_parse_manifest(world: &mut CliWorld, path: String) {
     parse_manifest_inner(world, &path);
 }
 
 #[when(expr = "the manifest file {string} is parsed")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn parse_manifest(world: &mut CliWorld, path: String) {
     parse_manifest_inner(world, &path);
 }
 
 #[when(regex = r"^the (?P<item>parsing result|manifest|version|flags|rules) (?:is|are) checked$")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn when_item_checked(world: &mut CliWorld, item: String) {
     match item.as_str() {
         "parsing result" => assert_parsed(world),
@@ -58,19 +66,22 @@ fn when_item_checked(world: &mut CliWorld, item: String) {
 }
 
 #[then(expr = "the manifest version is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn manifest_version(world: &mut CliWorld, version: String) {
     let manifest = world.manifest.as_ref().expect("manifest");
     assert_eq!(manifest.netsuke_version.to_string(), version);
 }
 
 #[then(expr = "the first target name is {string}")]
+#[allow(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn first_target_name(world: &mut CliWorld, name: String) {
-    let manifest = world.manifest.as_ref().expect("manifest");
-    let first = manifest.targets.first().expect("targets");
-    match &first.name {
-        StringOrList::String(value) => assert_eq!(value, &name),
-        other => panic!("Expected StringOrList::String, got: {other:?}"),
-    }
+    target_name_n(world, 1, name);
 }
 
 #[then("the first target is phony")]
@@ -100,6 +111,10 @@ fn manifest_parse_error(world: &mut CliWorld) {
 }
 
 #[then(expr = "the first rule name is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn first_rule_name(world: &mut CliWorld, name: String) {
     let manifest = world.manifest.as_ref().expect("manifest");
     let rule = manifest.rules.first().expect("rules");
@@ -107,14 +122,12 @@ fn first_rule_name(world: &mut CliWorld, name: String) {
 }
 
 #[then(expr = "the first target command is {string}")]
+#[allow(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn first_target_command(world: &mut CliWorld, command: String) {
-    let manifest = world.manifest.as_ref().expect("manifest");
-    let first = manifest.targets.first().expect("targets");
-    if let Recipe::Command { command: actual } = &first.recipe {
-        assert_eq!(actual, &command);
-    } else {
-        panic!("Expected command recipe, got: {:?}", first.recipe);
-    }
+    target_command_n(world, 1, command);
 }
 
 #[then(expr = "the manifest has {int} targets")]
@@ -124,6 +137,10 @@ fn manifest_has_targets(world: &mut CliWorld, count: usize) {
 }
 
 #[then(expr = "the target {int} name is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn target_name_n(world: &mut CliWorld, index: usize, name: String) {
     let manifest = world.manifest.as_ref().expect("manifest");
     let target = manifest
@@ -137,6 +154,10 @@ fn target_name_n(world: &mut CliWorld, index: usize, name: String) {
 }
 
 #[then(expr = "the target {int} command is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
 fn target_command_n(world: &mut CliWorld, index: usize, command: String) {
     let manifest = world.manifest.as_ref().expect("manifest");
     let target = manifest

From 1ca8b040327502524b0fa92af4ecbc0a45db24de Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Mon, 11 Aug 2025 11:41:05 +0100
Subject: [PATCH 3/3] Add yamllint ignore for Jinja fixtures

---
 .yamllint                        |  3 +++
 src/ast.rs                       |  7 +++--
 tests/data/jinja_if_disabled.yml |  6 +++++
 tests/features/manifest.feature  |  6 +++++
 tests/manifest_jinja_tests.rs    |  6 +++--
 tests/steps/manifest_steps.rs    | 44 +++++++++++++++++++-------------
 6 files changed, 50 insertions(+), 22 deletions(-)
 create mode 100644 .yamllint
 create mode 100644 tests/data/jinja_if_disabled.yml

diff --git a/.yamllint b/.yamllint
new file mode 100644
index 00000000..626837bc
--- /dev/null
+++ b/.yamllint
@@ -0,0 +1,3 @@
+extends: default
+ignore: |
+  tests/data/jinja_for*.yml
diff --git a/src/ast.rs b/src/ast.rs
index 2bf75673..5fd3fcc9 100644
--- a/src/ast.rs
+++ b/src/ast.rs
@@ -21,6 +21,9 @@ use semver::Version;
 use serde::{Deserialize, Serialize, de::Deserializer};
 use std::collections::HashMap;
 
+/// Map type for `vars` blocks, preserving YAML values.
+pub type Vars = HashMap<String, serde_yml::Value>;
+
 fn deserialize_actions<'de, D>(deserializer: D) -> Result<Vec<Target>, D::Error>
 where
     D: Deserializer<'de>,
@@ -61,7 +64,7 @@ pub struct NetsukeManifest {
 
     /// Global key/value pairs available to recipes.
     #[serde(default)]
-    pub vars: HashMap<String, serde_yml::Value>,
+    pub vars: Vars,
 
     /// Named rule templates that can be referenced by targets.
     #[serde(default)]
@@ -187,7 +190,7 @@ pub struct Target {
 
     /// Target-scoped variables available during command execution.
     #[serde(default)]
-    pub vars: HashMap<String, serde_yml::Value>,
+    pub vars: Vars,
 
     /// Declares that the target does not correspond to a real file.
     #[serde(default)]
diff --git a/tests/data/jinja_if_disabled.yml b/tests/data/jinja_if_disabled.yml
new file mode 100644
index 00000000..7a9c2c2e
--- /dev/null
+++ b/tests/data/jinja_if_disabled.yml
@@ -0,0 +1,6 @@
+netsuke_version: 1.0.0
+vars:
+  enable: false
+targets:
+  - name: hello
+    command: "{% if enable %}echo on{% else %}echo off{% endif %}"
diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature
index e20d3b22..f19be71f 100644
--- a/tests/features/manifest.feature
+++ b/tests/features/manifest.feature
@@ -62,6 +62,12 @@ Feature: Manifest Parsing
     Then the first target name is "hello"
     And the first target command is "echo on"
 
+  Scenario: Rendering Jinja conditionals in a manifest (disabled)
+    Given the manifest file "tests/data/jinja_if_disabled.yml" is parsed
+    When the manifest is checked
+    Then the first target name is "hello"
+    And the first target command is "echo off"
+
   Scenario: Rendering Jinja loops in a manifest
     Given the manifest file "tests/data/jinja_for.yml" is parsed
     When the manifest is checked
diff --git a/tests/manifest_jinja_tests.rs b/tests/manifest_jinja_tests.rs
index 14a940ea..824399cf 100644
--- a/tests/manifest_jinja_tests.rs
+++ b/tests/manifest_jinja_tests.rs
@@ -40,15 +40,17 @@ fn syntax_error_errors() {
 #[case(true, "echo on")]
 #[case(false, "echo off")]
 fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) {
+    let cmd = "{% if flag %}echo on{% else %}echo off{% endif %}";
     let yaml = manifest_yaml(&format!(
         concat!(
             "vars:\n",
             "  flag: {flag}\n",
             "targets:\n",
             "  - name: test\n",
-            "    command: {{% if flag %}}echo on{{% else %}}echo off{{% endif %}}\n",
+            "    command: {cmd}\n",
         ),
-        flag = flag
+        flag = flag,
+        cmd = cmd,
     ));
 
     let manifest = manifest::from_str(&yaml).expect("parse");
diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs
index 5b5c36c9..448a5a95 100644
--- a/tests/steps/manifest_steps.rs
+++ b/tests/steps/manifest_steps.rs
@@ -76,12 +76,12 @@ fn manifest_version(world: &mut CliWorld, version: String) {
 }
 
 #[then(expr = "the first target name is {string}")]
-#[allow(
+#[expect(
     clippy::needless_pass_by_value,
     reason = "Cucumber step requires owned String"
 )]
 fn first_target_name(world: &mut CliWorld, name: String) {
-    target_name_n(world, 1, name);
+    assert_target_name(world, 1, &name);
 }
 
 #[then("the first target is phony")]
@@ -122,12 +122,12 @@ fn first_rule_name(world: &mut CliWorld, name: String) {
 }
 
 #[then(expr = "the first target command is {string}")]
-#[allow(
+#[expect(
     clippy::needless_pass_by_value,
     reason = "Cucumber step requires owned String"
 )]
 fn first_target_command(world: &mut CliWorld, command: String) {
-    target_command_n(world, 1, command);
+    assert_target_command(world, 1, &command);
 }
 
 #[then(expr = "the manifest has {int} targets")]
@@ -136,37 +136,45 @@ fn manifest_has_targets(world: &mut CliWorld, count: usize) {
     assert_eq!(manifest.targets.len(), count);
 }
 
-#[then(expr = "the target {int} name is {string}")]
-#[expect(
-    clippy::needless_pass_by_value,
-    reason = "Cucumber step requires owned String"
-)]
-fn target_name_n(world: &mut CliWorld, index: usize, name: String) {
+fn assert_target_name(world: &CliWorld, index: usize, name: &str) {
     let manifest = world.manifest.as_ref().expect("manifest");
     let target = manifest
         .targets
         .get(index - 1)
         .unwrap_or_else(|| panic!("missing target {index}"));
     match &target.name {
-        StringOrList::String(value) => assert_eq!(value, &name),
+        StringOrList::String(value) => assert_eq!(value, name),
         other => panic!("Expected StringOrList::String, got: {other:?}"),
     }
 }
 
-#[then(expr = "the target {int} command is {string}")]
-#[expect(
-    clippy::needless_pass_by_value,
-    reason = "Cucumber step requires owned String"
-)]
-fn target_command_n(world: &mut CliWorld, index: usize, command: String) {
+fn assert_target_command(world: &CliWorld, index: usize, command: &str) {
     let manifest = world.manifest.as_ref().expect("manifest");
     let target = manifest
         .targets
         .get(index - 1)
         .unwrap_or_else(|| panic!("missing target {index}"));
     if let Recipe::Command { command: actual } = &target.recipe {
-        assert_eq!(actual, &command);
+        assert_eq!(actual, command);
     } else {
         panic!("Expected command recipe, got: {:?}", target.recipe);
     }
 }
+
+#[then(expr = "the target {int} name is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
+fn target_name_n(world: &mut CliWorld, index: usize, name: String) {
+    assert_target_name(world, index, &name);
+}
+
+#[then(expr = "the target {int} command is {string}")]
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber step requires owned String"
+)]
+fn target_command_n(world: &mut CliWorld, index: usize, command: String) {
+    assert_target_command(world, index, &command);
+}