From b56bc8b637a160f6a652c23166f9c313839d20e2 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:28:30 +0300
Subject: [PATCH 01/11] refactor(validate): extract per-node config validation
 into its own function

Move the large block of per-node config validation logic from `validate_all` into a dedicated `validate_node_configs` function, reducing the main validation function by over 300 lines and improving readability. The extracted function handles sub-workflow, fan-out, memory, dedup, and approval node config checks, keeping the same validation behaviour.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/validate.rs             | 326 +---------------------------------
 src/validate/node_config.rs | 344 ++++++++++++++++++++++++++++++++++++
 2 files changed, 345 insertions(+), 325 deletions(-)
 create mode 100644 src/validate/node_config.rs

diff --git a/src/validate.rs b/src/validate.rs
index 8935f75..e5d2651 100644
--- a/src/validate.rs
+++ b/src/validate.rs
@@ -142,331 +142,7 @@ pub fn validate_all(graph: &WorkflowGraph) -> Vec<ValidationError> {
         }
     }
 
-    // Per-kind config checks. A `sub_workflow` node must reference its child
-    // exactly one way: an inline `workflow` graph OR a `workflow_id` reference,
-    // never both and never neither (the reference form is resolved at run time
-    // via the host `WorkflowResolver`).
-    for node in &graph.nodes {
-        if node.kind == NodeKind::SubWorkflow {
-            let has_inline = node.config.get("workflow").is_some();
-            let has_ref = node
-                .config
-                .get("workflow_id")
-                .and_then(serde_json::Value::as_str)
-                .is_some_and(|s| !s.is_empty());
-            if has_inline == has_ref {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: "sub_workflow requires exactly one of `workflow` (inline) or \
-                             `workflow_id` (reference)"
-                        .to_string(),
-                });
-            }
-        }
-    }
-
-    // Per-item fan-out config (`execution` / `concurrency` / `on_item_error`).
-    // These select the execution strategy, so an unrecognized value cannot be
-    // caught at run time without silently changing behaviour — a bad
-    // `concurrency` would quietly stay sequential and a bad `on_item_error`
-    // would quietly pick a default. Reject them here, where the message can name
-    // the node.
-    for node in &graph.nodes {
-        let fans_out = matches!(
-            node.kind,
-            NodeKind::Agent
-                | NodeKind::ToolCall
-                | NodeKind::HttpRequest
-                | NodeKind::Memory
-                | NodeKind::SubWorkflow
-        );
-
-        if let Some(execution) = node.config.get("execution") {
-            match execution.as_str() {
-                Some("once" | "per_item") if fans_out => {}
-                Some("once" | "per_item") => {
-                    errors.push(ValidationError::InvalidNodeConfig {
-                        node: node.id.clone(),
-                        reason: format!(
-                            "`execution` is not supported on a {} node (only agent, tool_call, \
-                             http_request, memory, and sub_workflow map over their input)",
-                            kind_name(&node.kind)
-                        ),
-                    });
-                }
-                _ => {
-                    errors.push(ValidationError::InvalidNodeConfig {
-                        node: node.id.clone(),
-                        reason: format!(
-                            "unknown `execution` value {execution} (expected \"once\" or \
-                             \"per_item\")"
-                        ),
-                    });
-                }
-            }
-        }
-
-        // Whether this node actually maps over its input, accounting for the
-        // per-kind default: `tool_call` / `http_request` / `memory` are per-item
-        // unless told otherwise; `agent` / `sub_workflow` are not.
-        let per_item = match node.config.get("execution").and_then(Value::as_str) {
-            Some("per_item") => true,
-            Some("once") => false,
-            _ => matches!(
-                node.kind,
-                NodeKind::ToolCall | NodeKind::HttpRequest | NodeKind::Memory
-            ),
-        };
-
-        for key in ["concurrency", "on_item_error"] {
-            let Some(value) = node.config.get(key) else {
-                continue;
-            };
-            // A fan-out knob on a node that runs once is a no-op, and a silent
-            // no-op reads as "I asked for parallelism and got none".
-            if !per_item {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!(
-                        "`{key}` has no effect without `execution: \"per_item\"` on a {} node",
-                        kind_name(&node.kind)
-                    ),
-                });
-                continue;
-            }
-            let ok = match key {
-                "concurrency" => {
-                    matches!(
-                        value,
-                        Value::Number(n) if n.as_u64().is_some(),
-                    ) || value.as_str() == Some("all")
-                }
-                _ => matches!(value.as_str(), Some("collect" | "fail_fast" | "skip")),
-            };
-            if !ok {
-                let expected = if key == "concurrency" {
-                    "a non-negative integer or \"all\""
-                } else {
-                    "\"collect\", \"fail_fast\", or \"skip\""
-                };
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!("`{key}` must be {expected}, got {value}"),
-                });
-            }
-        }
-    }
-
-    // `memory` node config checks, including THE hard security invariant: a
-    // `remember`/`forget` operation may never target `scope: "user"` — the
-    // caller's durable, cross-flow memory. Rejecting this structurally, at the
-    // door, means a workflow (or an LLM authoring one) can never plant or erase
-    // durable facts about the user by way of a `remember`/`forget` node; the
-    // only scope those two operations may write through is `"flow"`.
-    for node in &graph.nodes {
-        if node.kind != NodeKind::Memory {
-            continue;
-        }
-
-        let operation = node.config.get("operation").and_then(Value::as_str);
-        let Some(operation) = operation else {
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: "memory node requires `operation` (recall|search|flavour|people|\
-                         remember|forget)"
-                    .to_string(),
-            });
-            continue;
-        };
-        if !matches!(
-            operation,
-            "recall" | "search" | "flavour" | "people" | "remember" | "forget"
-        ) {
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: format!(
-                    "memory node has unknown operation {operation:?} (expected one of \
-                     recall|search|flavour|people|remember|forget)"
-                ),
-            });
-            continue;
-        }
-
-        let scope = node.config.get("scope").and_then(Value::as_str);
-
-        // THE hard invariant (see the block comment above): reject before any
-        // other config check, so it can never be masked by a different error.
-        // remember/forget may write ONLY scope "flow". BOTH read-only scopes are
-        // rejected here — "user" (the user's durable memory) and "flows"
-        // (cross-flow read). This gate is unbypassable precisely because `scope`
-        // is validated as a literal enum (below): an "=expr" binding resolves at
-        // runtime and is never one of user|flow|flows, so it fails the enum
-        // check and can never smuggle a write past this into
-        // provider.remember/forget. If a future change makes `scope` bindable,
-        // this invariant reopens — keep the enum check.
-        if matches!(operation, "remember" | "forget")
-            && matches!(scope, Some("user") | Some("flows"))
-        {
-            let bad = scope.unwrap_or_default();
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: format!(
-                    "memory node operation {operation:?} may not target scope {bad:?} — \
-                     remember/forget may only write scope \"flow\"; scopes \"user\" and \
-                     \"flows\" are read-only from a workflow"
-                ),
-            });
-        }
-
-        if let Some(scope) = scope {
-            if !matches!(scope, "user" | "flow" | "flows") {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!(
-                        "memory node has unknown scope {scope:?} (expected \
-                         user|flow|flows)"
-                    ),
-                });
-            }
-        }
-
-        // `scope` is required for recall/remember/forget (not search/flavour/
-        // people — see the catalog contract for the exact per-operation table).
-        if matches!(operation, "recall" | "remember" | "forget") && scope.is_none() {
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: format!("memory node operation {operation:?} requires `scope`"),
-            });
-        }
-
-        if matches!(operation, "recall" | "search") {
-            let has_query = node
-                .config
-                .get("query")
-                .and_then(Value::as_str)
-                .is_some_and(|s| !s.is_empty());
-            if !has_query {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!("memory node operation {operation:?} requires `query`"),
-                });
-            }
-        }
-
-        if operation == "flavour" {
-            let has_flavour = node
-                .config
-                .get("flavour")
-                .and_then(Value::as_str)
-                .is_some_and(|s| !s.is_empty());
-            if !has_flavour {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: "memory node operation \"flavour\" requires `flavour` (slug)"
-                        .to_string(),
-                });
-            }
-        }
-
-        if matches!(operation, "remember" | "forget") {
-            let has_key = node
-                .config
-                .get("key")
-                .and_then(Value::as_str)
-                .is_some_and(|s| !s.is_empty());
-            if !has_key {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!("memory node operation {operation:?} requires `key`"),
-                });
-            }
-        }
-
-        if operation == "remember" && node.config.get("value").is_none() {
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: "memory node operation \"remember\" requires `value`".to_string(),
-            });
-        }
-    }
-
-    // `dedup` node config checks: `key` (the per-item "=expr" dedup key) is the
-    // only config field, and it is required — a dedup node with no `key` can
-    // never resolve anything to compare, which is always an authoring mistake
-    // (as opposed to a `key` that *resolves* to null at run time, which is the
-    // intentional, per-item fail-open path the executor handles).
-    for node in &graph.nodes {
-        if node.kind != NodeKind::Dedup {
-            continue;
-        }
-        let has_key = node
-            .config
-            .get("key")
-            .and_then(Value::as_str)
-            .is_some_and(|s| !s.is_empty());
-        if !has_key {
-            errors.push(ValidationError::InvalidNodeConfig {
-                node: node.id.clone(),
-                reason: "dedup node requires `key` (an \"=expr\" resolved per item, e.g. \
-                         \"=item.id\")"
-                    .to_string(),
-            });
-        }
-    }
-
-    // `approval` node config. These are all closed enums that SELECT BEHAVIOUR,
-    // so a typo cannot be caught at run time without silently changing what the
-    // node does: a misspelled `on_reject` would quietly route a rejection that
-    // was meant to fail the run, and a misspelled `wait_mode` would quietly
-    // suspend a review the author wanted polled. Refuse them at the door, where
-    // the message can name the node and the alternatives.
-    for node in &graph.nodes {
-        if node.kind != NodeKind::Approval {
-            continue;
-        }
-
-        for (key, allowed) in [
-            ("wait_mode", &["suspend", "poll"][..]),
-            ("on_reject", &["route", "error", "drop"][..]),
-            ("on_timeout", &["error", "reject", "route"][..]),
-        ] {
-            let Some(value) = node.config.get(key) else {
-                continue;
-            };
-            if !value.as_str().is_some_and(|v| allowed.contains(&v)) {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: format!(
-                        "approval node has unknown `{key}` {value} (expected one of {})",
-                        allowed
-                            .iter()
-                            .map(|v| format!("{v:?}"))
-                            .collect::<Vec<_>>()
-                            .join(", ")
-                    ),
-                });
-            }
-        }
-
-        // Reviewer handles are opaque to the crate, but their *shape* is not:
-        // a bare string here (the natural mistake for a single reviewer) would
-        // be read as "nobody", and the review would go to an empty audience
-        // with no error anywhere. An empty array reaches the same audience of
-        // nobody just as silently, so it is refused for the same reason.
-        if let Some(assignees) = node.config.get("assignees") {
-            if !assignees
-                .as_array()
-                .is_some_and(|values| !values.is_empty() && values.iter().all(Value::is_string))
-            {
-                errors.push(ValidationError::InvalidNodeConfig {
-                    node: node.id.clone(),
-                    reason: "approval node `assignees` must be a non-empty array of strings (a \
-                             single reviewer is a one-element array)"
-                        .to_string(),
-                });
-            }
-        }
-    }
+    validate_node_configs(graph, &mut errors);
 
     // `void` node topology checks. The kind asserts exactly one thing — "the
     // branch ends here, deliberately" — so the two ways to contradict it are
diff --git a/src/validate/node_config.rs b/src/validate/node_config.rs
new file mode 100644
index 0000000..6f0ac2d
--- /dev/null
+++ b/src/validate/node_config.rs
@@ -0,0 +1,344 @@
+//! Per-kind node **config** validation.
+//!
+//! The half of [`validate_all`](super::validate_all) that reads a node's
+//! `config` object rather than the graph's shape: the `sub_workflow` child
+//! reference, per-item fan-out selectors, `memory` scope rules, the `dedup`
+//! key, and the `approval` enums. Every check here is about one node in
+//! isolation — nothing in this module looks at an edge — which is what makes it
+//! a module of its own rather than an arbitrary cut through `validate_all`.
+
+use serde_json::Value;
+
+use crate::error::ValidationError;
+use crate::model::{NodeKind, WorkflowGraph};
+
+use super::kind_name;
+
+/// Appends every per-kind config error the graph carries, in node order.
+pub(super) fn validate_node_configs(graph: &WorkflowGraph, errors: &mut Vec<ValidationError>) {
+    // Per-kind config checks. A `sub_workflow` node must reference its child
+    // exactly one way: an inline `workflow` graph OR a `workflow_id` reference,
+    // never both and never neither (the reference form is resolved at run time
+    // via the host `WorkflowResolver`).
+    for node in &graph.nodes {
+        if node.kind == NodeKind::SubWorkflow {
+            let has_inline = node.config.get("workflow").is_some();
+            let has_ref = node
+                .config
+                .get("workflow_id")
+                .and_then(serde_json::Value::as_str)
+                .is_some_and(|s| !s.is_empty());
+            if has_inline == has_ref {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: "sub_workflow requires exactly one of `workflow` (inline) or \
+                             `workflow_id` (reference)"
+                        .to_string(),
+                });
+            }
+        }
+    }
+
+    // Per-item fan-out config (`execution` / `concurrency` / `on_item_error`).
+    // These select the execution strategy, so an unrecognized value cannot be
+    // caught at run time without silently changing behaviour — a bad
+    // `concurrency` would quietly stay sequential and a bad `on_item_error`
+    // would quietly pick a default. Reject them here, where the message can name
+    // the node.
+    for node in &graph.nodes {
+        let fans_out = matches!(
+            node.kind,
+            NodeKind::Agent
+                | NodeKind::ToolCall
+                | NodeKind::HttpRequest
+                | NodeKind::Memory
+                | NodeKind::SubWorkflow
+        );
+
+        if let Some(execution) = node.config.get("execution") {
+            match execution.as_str() {
+                Some("once" | "per_item") if fans_out => {}
+                Some("once" | "per_item") => {
+                    errors.push(ValidationError::InvalidNodeConfig {
+                        node: node.id.clone(),
+                        reason: format!(
+                            "`execution` is not supported on a {} node (only agent, tool_call, \
+                             http_request, memory, and sub_workflow map over their input)",
+                            kind_name(&node.kind)
+                        ),
+                    });
+                }
+                _ => {
+                    errors.push(ValidationError::InvalidNodeConfig {
+                        node: node.id.clone(),
+                        reason: format!(
+                            "unknown `execution` value {execution} (expected \"once\" or \
+                             \"per_item\")"
+                        ),
+                    });
+                }
+            }
+        }
+
+        // Whether this node actually maps over its input, accounting for the
+        // per-kind default: `tool_call` / `http_request` / `memory` are per-item
+        // unless told otherwise; `agent` / `sub_workflow` are not.
+        let per_item = match node.config.get("execution").and_then(Value::as_str) {
+            Some("per_item") => true,
+            Some("once") => false,
+            _ => matches!(
+                node.kind,
+                NodeKind::ToolCall | NodeKind::HttpRequest | NodeKind::Memory
+            ),
+        };
+
+        for key in ["concurrency", "on_item_error"] {
+            let Some(value) = node.config.get(key) else {
+                continue;
+            };
+            // A fan-out knob on a node that runs once is a no-op, and a silent
+            // no-op reads as "I asked for parallelism and got none".
+            if !per_item {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!(
+                        "`{key}` has no effect without `execution: \"per_item\"` on a {} node",
+                        kind_name(&node.kind)
+                    ),
+                });
+                continue;
+            }
+            let ok = match key {
+                "concurrency" => {
+                    matches!(
+                        value,
+                        Value::Number(n) if n.as_u64().is_some(),
+                    ) || value.as_str() == Some("all")
+                }
+                _ => matches!(value.as_str(), Some("collect" | "fail_fast" | "skip")),
+            };
+            if !ok {
+                let expected = if key == "concurrency" {
+                    "a non-negative integer or \"all\""
+                } else {
+                    "\"collect\", \"fail_fast\", or \"skip\""
+                };
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!("`{key}` must be {expected}, got {value}"),
+                });
+            }
+        }
+    }
+
+    // `memory` node config checks, including THE hard security invariant: a
+    // `remember`/`forget` operation may never target `scope: "user"` — the
+    // caller's durable, cross-flow memory. Rejecting this structurally, at the
+    // door, means a workflow (or an LLM authoring one) can never plant or erase
+    // durable facts about the user by way of a `remember`/`forget` node; the
+    // only scope those two operations may write through is `"flow"`.
+    for node in &graph.nodes {
+        if node.kind != NodeKind::Memory {
+            continue;
+        }
+
+        let operation = node.config.get("operation").and_then(Value::as_str);
+        let Some(operation) = operation else {
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: "memory node requires `operation` (recall|search|flavour|people|\
+                         remember|forget)"
+                    .to_string(),
+            });
+            continue;
+        };
+        if !matches!(
+            operation,
+            "recall" | "search" | "flavour" | "people" | "remember" | "forget"
+        ) {
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: format!(
+                    "memory node has unknown operation {operation:?} (expected one of \
+                     recall|search|flavour|people|remember|forget)"
+                ),
+            });
+            continue;
+        }
+
+        let scope = node.config.get("scope").and_then(Value::as_str);
+
+        // THE hard invariant (see the block comment above): reject before any
+        // other config check, so it can never be masked by a different error.
+        // remember/forget may write ONLY scope "flow". BOTH read-only scopes are
+        // rejected here — "user" (the user's durable memory) and "flows"
+        // (cross-flow read). This gate is unbypassable precisely because `scope`
+        // is validated as a literal enum (below): an "=expr" binding resolves at
+        // runtime and is never one of user|flow|flows, so it fails the enum
+        // check and can never smuggle a write past this into
+        // provider.remember/forget. If a future change makes `scope` bindable,
+        // this invariant reopens — keep the enum check.
+        if matches!(operation, "remember" | "forget")
+            && matches!(scope, Some("user") | Some("flows"))
+        {
+            let bad = scope.unwrap_or_default();
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: format!(
+                    "memory node operation {operation:?} may not target scope {bad:?} — \
+                     remember/forget may only write scope \"flow\"; scopes \"user\" and \
+                     \"flows\" are read-only from a workflow"
+                ),
+            });
+        }
+
+        if let Some(scope) = scope {
+            if !matches!(scope, "user" | "flow" | "flows") {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!(
+                        "memory node has unknown scope {scope:?} (expected \
+                         user|flow|flows)"
+                    ),
+                });
+            }
+        }
+
+        // `scope` is required for recall/remember/forget (not search/flavour/
+        // people — see the catalog contract for the exact per-operation table).
+        if matches!(operation, "recall" | "remember" | "forget") && scope.is_none() {
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: format!("memory node operation {operation:?} requires `scope`"),
+            });
+        }
+
+        if matches!(operation, "recall" | "search") {
+            let has_query = node
+                .config
+                .get("query")
+                .and_then(Value::as_str)
+                .is_some_and(|s| !s.is_empty());
+            if !has_query {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!("memory node operation {operation:?} requires `query`"),
+                });
+            }
+        }
+
+        if operation == "flavour" {
+            let has_flavour = node
+                .config
+                .get("flavour")
+                .and_then(Value::as_str)
+                .is_some_and(|s| !s.is_empty());
+            if !has_flavour {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: "memory node operation \"flavour\" requires `flavour` (slug)"
+                        .to_string(),
+                });
+            }
+        }
+
+        if matches!(operation, "remember" | "forget") {
+            let has_key = node
+                .config
+                .get("key")
+                .and_then(Value::as_str)
+                .is_some_and(|s| !s.is_empty());
+            if !has_key {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!("memory node operation {operation:?} requires `key`"),
+                });
+            }
+        }
+
+        if operation == "remember" && node.config.get("value").is_none() {
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: "memory node operation \"remember\" requires `value`".to_string(),
+            });
+        }
+    }
+
+    // `dedup` node config checks: `key` (the per-item "=expr" dedup key) is the
+    // only config field, and it is required — a dedup node with no `key` can
+    // never resolve anything to compare, which is always an authoring mistake
+    // (as opposed to a `key` that *resolves* to null at run time, which is the
+    // intentional, per-item fail-open path the executor handles).
+    for node in &graph.nodes {
+        if node.kind != NodeKind::Dedup {
+            continue;
+        }
+        let has_key = node
+            .config
+            .get("key")
+            .and_then(Value::as_str)
+            .is_some_and(|s| !s.is_empty());
+        if !has_key {
+            errors.push(ValidationError::InvalidNodeConfig {
+                node: node.id.clone(),
+                reason: "dedup node requires `key` (an \"=expr\" resolved per item, e.g. \
+                         \"=item.id\")"
+                    .to_string(),
+            });
+        }
+    }
+
+    // `approval` node config. These are all closed enums that SELECT BEHAVIOUR,
+    // so a typo cannot be caught at run time without silently changing what the
+    // node does: a misspelled `on_reject` would quietly route a rejection that
+    // was meant to fail the run, and a misspelled `wait_mode` would quietly
+    // suspend a review the author wanted polled. Refuse them at the door, where
+    // the message can name the node and the alternatives.
+    for node in &graph.nodes {
+        if node.kind != NodeKind::Approval {
+            continue;
+        }
+
+        for (key, allowed) in [
+            ("wait_mode", &["suspend", "poll"][..]),
+            ("on_reject", &["route", "error", "drop"][..]),
+            ("on_timeout", &["error", "reject", "route"][..]),
+        ] {
+            let Some(value) = node.config.get(key) else {
+                continue;
+            };
+            if !value.as_str().is_some_and(|v| allowed.contains(&v)) {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: format!(
+                        "approval node has unknown `{key}` {value} (expected one of {})",
+                        allowed
+                            .iter()
+                            .map(|v| format!("{v:?}"))
+                            .collect::<Vec<_>>()
+                            .join(", ")
+                    ),
+                });
+            }
+        }
+
+        // Reviewer handles are opaque to the crate, but their *shape* is not:
+        // a bare string here (the natural mistake for a single reviewer) would
+        // be read as "nobody", and the review would go to an empty audience
+        // with no error anywhere. An empty array reaches the same audience of
+        // nobody just as silently, so it is refused for the same reason.
+        if let Some(assignees) = node.config.get("assignees") {
+            if !assignees
+                .as_array()
+                .is_some_and(|values| !values.is_empty() && values.iter().all(Value::is_string))
+            {
+                errors.push(ValidationError::InvalidNodeConfig {
+                    node: node.id.clone(),
+                    reason: "approval node `assignees` must be a non-empty array of strings (a \
+                             single reviewer is a one-element array)"
+                        .to_string(),
+                });
+            }
+        }
+    }
+}

From 5eadf6d53f3f942fa4cbbb69f3cbd4e425359a00 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:29:15 +0300
Subject: [PATCH 02/11] feat(validate): add node configuration validation

Introduce validation for node configurations by adding a new module and importing its validation function, ensuring that node configs are checked alongside existing validations.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/validate.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/validate.rs b/src/validate.rs
index e5d2651..d7b311c 100644
--- a/src/validate.rs
+++ b/src/validate.rs
@@ -255,6 +255,9 @@ use agents::validate_agents;
 mod loops;
 use loops::validate_loops;
 
+mod node_config;
+use node_config::validate_node_configs;
+
 mod scatter;
 use scatter::{nodes_on_cycle, path_exists, validate_scatter_regions};
 

From a4e402f07f579f6b95ae2283c06977cc99249762 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:29:33 +0300
Subject: [PATCH 03/11] fix(agent): reject null cwd or working_dir instead of
 silently falling back

A null value for `cwd` or `working_dir` now fails the node with a clear error, rather than being treated as if the key were absent. This prevents a step from silently running in a different directory when an expression resolves to null because the upstream node failed or a key moved.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/nodes/integration/agent_request.rs | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/nodes/integration/agent_request.rs b/src/nodes/integration/agent_request.rs
index 4df4d05..d6427f5 100644
--- a/src/nodes/integration/agent_request.rs
+++ b/src/nodes/integration/agent_request.rs
@@ -191,14 +191,29 @@ fn narrow_tools(granted: &[ToolGrant], requested: &[ToolGrant], node_id: &str) -
 /// both are set.
 ///
 /// # Errors
-/// Refuses a non-string or a blank value, exactly as a `shell` node's `cwd`
-/// does. A number or an empty string here is an authoring slip, and the
-/// alternative is a step that silently runs somewhere else.
+/// Refuses a non-string, a blank value, **or a null**, exactly as a `shell`
+/// node's `cwd` does. A number or an empty string here is an authoring slip,
+/// and the alternative is a step that silently runs somewhere else.
+///
+/// Null is the one worth spelling out. `cfg` arrives already
+/// expression-resolved, so `"cwd": "=nodes.prepare.item.json.worktree"` becomes
+/// `null` whenever that path is missing — the upstream node failed, or the key
+/// moved. Treating that as "no `cwd` declared" would fall through to
+/// `working_dir`, then to the definition's own directory, then to whatever the
+/// harness defaults to: the step runs in a *different checkout* and says
+/// nothing. A directory an author named is never silently swapped for another,
+/// so a present-but-null value fails the node instead.
 pub(crate) fn declared_working_dir(cfg: &Value, node_id: &str) -> Result<Option<String>> {
     for key in ["cwd", "working_dir"] {
-        let Some(value) = cfg.get(key).filter(|v| !v.is_null()) else {
+        let Some(value) = cfg.get(key) else {
             continue;
         };
+        if value.is_null() {
+            return Err(EngineError::Capability(format!(
+                "agent node {node_id}: `{key}` resolved to null; an expression that reads a \
+                 missing path fails the step rather than falling back to another directory"
+            )));
+        }
         let dir = value.as_str().ok_or_else(|| {
             EngineError::Capability(format!("agent node {node_id}: `{key}` must be a string"))
         })?;

From 4776c40b2044fd012d1bd5c3e2fc2d6ecef0ae50 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:30:11 +0300
Subject: [PATCH 04/11] test(agent-workdir): add e2e tests for null cwd
 resolution

Add two end-to-end tests that verify the agent correctly fails a step when the `cwd` expression resolves to `null`, rather than silently falling back to the working directory or treating the value as absent.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 tests/agent_workdir_e2e.rs | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tests/agent_workdir_e2e.rs b/tests/agent_workdir_e2e.rs
index 53a668d..6669908 100644
--- a/tests/agent_workdir_e2e.rs
+++ b/tests/agent_workdir_e2e.rs
@@ -146,6 +146,42 @@ async fn a_cwd_that_does_not_exist_fails_naming_the_path() {
     );
 }
 
+#[tokio::test]
+async fn a_cwd_expression_that_resolves_to_null_fails_the_step() {
+    // The upstream node did not publish the key the `cwd` expression reads, so
+    // the resolved config carries `null`. That must fail here rather than read
+    // as "no `cwd` declared" and let the harness pick its own directory.
+    let root = workspace();
+    let graph = parse(graph_json(
+        &canonical(&root),
+        json!("=nodes.prepare.item.missing_key"),
+    ));
+
+    let error = run_graph(&graph).await.expect_err("the step must fail");
+
+    assert!(error.contains("resolved to null"), "{error}");
+    assert!(error.contains("agent node code"), "{error}");
+}
+
+#[tokio::test]
+async fn a_null_cwd_does_not_fall_back_to_working_dir() {
+    // Both spellings present, `cwd` resolving to null: the older `working_dir`
+    // must not quietly win. Picking it up would run the step in a directory the
+    // author's `cwd` expression was meant to override.
+    let root = workspace();
+    let mut graph = graph_json(&canonical(&root), json!("=nodes.prepare.item.missing_key"));
+    graph["nodes"][2]["config"]["working_dir"] = json!("worktrees/issue-1");
+    let graph = parse(graph);
+
+    let error = run_graph(&graph).await.expect_err("the step must fail");
+
+    assert!(error.contains("resolved to null"), "{error}");
+    assert!(
+        !error.contains("worktrees/issue-1"),
+        "the fallback is never reached: {error}"
+    );
+}
+
 #[tokio::test]
 async fn a_run_with_no_workspace_passes_the_directory_through_unchanged() {
     // Back-compat: a harness whose agents run in a sandbox names directories

From d4d53d912e9ed82c0584c8fd9c0b3876581ea767 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:31:22 +0300
Subject: [PATCH 05/11] feat(agent): add WorkdirCheck enum and resolve_workdir
 method

Introduces a new public enum `WorkdirCheck` and a default method `resolve_workdir` on the `AgentRunner` trait, allowing hosts to resolve and validate a node's declared working directory against their own filesystem. The engine previously had no way to check path existence or containment on a remote or sandboxed filesystem, so this change delegates that responsibility to the host while preserving backward compatibility through a default implementation that returns `Unmanaged`.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/caps/agent.rs        |  2 +-
 src/caps/agent/runner.rs | 45 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/src/caps/agent.rs b/src/caps/agent.rs
index 377c04d..5afe2c9 100644
--- a/src/caps/agent.rs
+++ b/src/caps/agent.rs
@@ -433,7 +433,7 @@ impl AgentRunOutcome {
 }
 
 mod runner;
-pub use runner::AgentRunner;
+pub use runner::{AgentRunner, WorkdirCheck};
 
 #[cfg(test)]
 #[path = "agent_tests.rs"]
diff --git a/src/caps/agent/runner.rs b/src/caps/agent/runner.rs
index 7064fac..d1baee5 100644
--- a/src/caps/agent/runner.rs
+++ b/src/caps/agent/runner.rs
@@ -1,5 +1,22 @@
 use super::*;
 
+/// A host's answer to "where does this declared working directory actually
+/// live?" — see [`AgentRunner::resolve_workdir`].
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum WorkdirCheck {
+    /// Not this harness's filesystem to judge. The engine falls back to
+    /// checking the directory on its own process filesystem, which is the
+    /// behavior every host had before this method existed.
+    Unmanaged,
+    /// The directory exists in that workspace, and this is its canonical path
+    /// on the filesystem the agent will actually run on. The engine passes it
+    /// to the harness verbatim.
+    Resolved(String),
+    /// The directory is refused, for this reason. The engine fails the step
+    /// with the message, prefixed with the node it came from.
+    Refused(String),
+}
+
 /// Runs a host-registered, multi-turn **agent** — the harness seam.
 ///
 /// Where [`LlmProvider::complete`](crate::caps::LlmProvider::complete) is a
@@ -181,4 +198,32 @@ pub trait AgentRunner: Send + Sync {
             .map(|grant| ToolDescriptor::from_grant(grant, conn))
             .collect())
     }
+
+    /// Resolves a node's declared working directory (`config.cwd`, a
+    /// `sub_workflow` node's `config.workspace`) against `workspace`, on the
+    /// filesystem the agent will actually run on.
+    ///
+    /// The engine has no filesystem of its own. It can, and does, check the
+    /// *shape* of a declared directory — an absolute path, a `..` traversal —
+    /// because that is string arithmetic. Deciding whether the path **exists**,
+    /// what it canonicalizes to, and whether it is a directory is an
+    /// outside-world effect, and on a harness whose agents run in a remote
+    /// sandbox or a container the answer is not on the engine's disk at all.
+    /// This method is where such a host answers for its own filesystem.
+    ///
+    /// `workspace` is the run's declared boundary and `declared` the raw value
+    /// the author wrote (already expression-resolved). A host that resolves the
+    /// pair must also **contain** it: returning
+    /// [`Resolved`](WorkdirCheck::Resolved) asserts the path is inside
+    /// `workspace`, since only the host can compare two paths on its own
+    /// filesystem.
+    ///
+    /// The default returns [`Unmanaged`](WorkdirCheck::Unmanaged) for
+    /// everything, so the engine canonicalizes locally exactly as it did before
+    /// this method existed. A host whose agents share the engine's filesystem
+    /// wants precisely that and should leave it alone.
+    async fn resolve_workdir(&self, workspace: &str, declared: &str) -> WorkdirCheck {
+        let _ = (workspace, declared);
+        WorkdirCheck::Unmanaged
+    }
 }

From 3fb41c8ae8db77b428965ad74771247983984492 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:31:36 +0300
Subject: [PATCH 06/11] fix(caps): re-export WorkdirCheck from agent module

The `WorkdirCheck` type was added to the agent module but not re-exported from the caps module, making it inaccessible to external consumers. This change adds it to the public re-export list so it can be used by callers of the caps API.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/caps/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/caps/mod.rs b/src/caps/mod.rs
index 4e4bb40..4f43215 100644
--- a/src/caps/mod.rs
+++ b/src/caps/mod.rs
@@ -24,7 +24,7 @@ use crate::error::Result;
 
 pub use self::agent::{
     AgentInput, AgentModelSelection, AgentRunIdentity, AgentRunOutcome, AgentRunRequest,
-    AgentRunner, AgentUsage, ContextBlock, StopReason, ToolDescriptor,
+    AgentRunner, AgentUsage, ContextBlock, StopReason, ToolDescriptor, WorkdirCheck,
 };
 pub use self::approval::{
     ApprovalDecision, ApprovalOutcome, ApprovalProvider, ApprovalRequest, ApprovalSubject,

From e3d55ef9b6b27b0caf848d4721ca33e2f8c4e821 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:32:16 +0300
Subject: [PATCH 07/11] feat(workdir): split path validation into shape and
 filesystem checks

Extract the filesystem-independent shape validation (absolute-vs-relative and `..` traversal) into a new `check_shape` function that runs before any filesystem access, and route the existence and directory checks through `AgentRunner::resolve_workdir` when a harness claims the workspace. This ensures the containment check that matters most cannot be accidentally dropped by a host implementation running agents on a remote filesystem.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/workdir.rs | 103 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 83 insertions(+), 20 deletions(-)

diff --git a/src/workdir.rs b/src/workdir.rs
index 50d564c..1925b0a 100644
--- a/src/workdir.rs
+++ b/src/workdir.rs
@@ -22,6 +22,27 @@
 //! a host whose agents run in a remote sandbox names directories the engine's
 //! own filesystem knows nothing about, and checking those against the local disk
 //! would fail every one of them.
+//!
+//! # Whose filesystem
+//!
+//! A run that *does* declare a workspace still may not be running its agents on
+//! this process's disk. The checks therefore split in two:
+//!
+//! - **Shape** — absolute-vs-relative, `..` traversal — is string arithmetic
+//!   with no filesystem in it, so the engine always does it, first, and no host
+//!   can weaken it.
+//! - **Existence, canonical form, and directory-ness** are outside-world
+//!   effects, and they route through
+//!   [`AgentRunner::resolve_workdir`](crate::caps::AgentRunner::resolve_workdir).
+//!   A harness that owns a remote or containerized workspace answers for it;
+//!   the default answer is
+//!   [`Unmanaged`](crate::caps::WorkdirCheck::Unmanaged), which falls back to
+//!   the engine's own filesystem exactly as before.
+//!
+//! The shell node reached the same shape by a different route: it hands
+//! `args.cwd` to the [`ShellRunner`](crate::caps::ShellRunner) untouched and the
+//! host's [`ScriptPolicy`](crate::caps::host::ScriptPolicy) contains it. This
+//! module is the `agent`/`sub_workflow` equivalent.
 
 use std::path::{Component, Path, PathBuf};
 
@@ -61,6 +82,40 @@ pub(crate) fn run_workspace(run: &Value) -> Option<&str> {
         .filter(|w| !w.is_empty())
 }
 
+/// The filesystem-free half of the rule: is this value even *shaped* like a
+/// path inside a workspace?
+///
+/// Absolute-vs-relative and `..` traversal are decided by reading the string,
+/// so the engine answers them itself on every host — including one whose agents
+/// run somewhere it cannot see. Keeping this here rather than behind
+/// [`AgentRunner::resolve_workdir`](crate::caps::AgentRunner::resolve_workdir)
+/// means a host implementation cannot accidentally drop the containment check
+/// that matters most.
+///
+/// # Errors
+/// Returns the refusal message when `raw` is absolute under
+/// [`Absolute::Refuse`], or when a relative path traverses upwards.
+pub(crate) fn check_shape(raw: &str, field: &str, absolute: Absolute) -> Result<(), String> {
+    let candidate = Path::new(raw);
+    if candidate.is_absolute() {
+        if absolute == Absolute::Refuse {
+            return Err(format!(
+                "`{field}` ('{raw}') must be relative to the workspace, not absolute"
+            ));
+        }
+    } else if candidate.components().any(|component| {
+        matches!(
+            component,
+            Component::ParentDir | Component::RootDir | Component::Prefix(_)
+        )
+    }) {
+        return Err(format!(
+            "`{field}` ('{raw}') must not traverse outside the workspace"
+        ));
+    }
+    Ok(())
+}
+
 /// Resolves `raw` against `workspace`, refusing anything that escapes it.
 ///
 /// Both halves are load-bearing, and both come from the shell step's policy.
@@ -83,23 +138,8 @@ pub(crate) fn resolve_in_workspace(
     field: &str,
     absolute: Absolute,
 ) -> Result<PathBuf, String> {
+    check_shape(raw, field, absolute)?;
     let candidate = Path::new(raw);
-    if candidate.is_absolute() {
-        if absolute == Absolute::Refuse {
-            return Err(format!(
-                "`{field}` ('{raw}') must be relative to the workspace, not absolute"
-            ));
-        }
-    } else if candidate.components().any(|component| {
-        matches!(
-            component,
-            Component::ParentDir | Component::RootDir | Component::Prefix(_)
-        )
-    }) {
-        return Err(format!(
-            "`{field}` ('{raw}') must not traverse outside the workspace"
-        ));
-    }
 
     let workspace = workspace.canonicalize().map_err(|err| {
         format!(
@@ -157,11 +197,17 @@ pub(crate) fn resolve_dir_in_workspace(
 /// `raw` unchanged when it is not (see the module docs: the engine cannot check
 /// a directory on a filesystem it does not have).
 ///
+/// The shape check runs first and always. Existence and directory-ness go to
+/// [`AgentRunner::resolve_workdir`](crate::caps::AgentRunner::resolve_workdir)
+/// when a harness is wired and claims the workspace, and to this process's
+/// filesystem otherwise.
+///
 /// # Errors
 /// Returns [`EngineError::Capability`](crate::error::EngineError::Capability),
 /// prefixed with `surface`, when the directory escapes the workspace, does not
 /// exist, or is not a directory.
-pub(crate) fn resolve_node_dir(
+pub(crate) async fn resolve_node_dir(
+    agent: Option<&std::sync::Arc<dyn crate::caps::AgentRunner>>,
     run: &Value,
     raw: &str,
     field: &str,
@@ -175,10 +221,27 @@ pub(crate) fn resolve_node_dir(
         );
         return Ok(raw.to_string());
     };
+    let refuse =
+        |message: String| crate::error::EngineError::Capability(format!("{surface}: {message}"));
+
+    check_shape(raw, field, Absolute::AllowInside).map_err(refuse)?;
+
+    if let Some(runner) = agent {
+        match runner.resolve_workdir(workspace, raw).await {
+            crate::caps::WorkdirCheck::Resolved(path) => {
+                tracing::debug!(field, raw, %path, "workdir: resolved by the agent harness");
+                return Ok(path);
+            }
+            crate::caps::WorkdirCheck::Refused(message) => {
+                return Err(refuse(format!("`{field}` ('{raw}') {message}")));
+            }
+            crate::caps::WorkdirCheck::Unmanaged => {}
+        }
+    }
+
     let resolved =
-        resolve_dir_in_workspace(Path::new(workspace), raw, field, Absolute::AllowInside).map_err(
-            |message| crate::error::EngineError::Capability(format!("{surface}: {message}")),
-        )?;
+        resolve_dir_in_workspace(Path::new(workspace), raw, field, Absolute::AllowInside)
+            .map_err(refuse)?;
     Ok(resolved.to_string_lossy().into_owned())
 }
 

From b6b4251d38d9743cc089bf447298d60f21ba5453 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:33:16 +0300
Subject: [PATCH 08/11] fix(agent): make working-dir resolution async

The `resolve_working_dir` function and `child_workspace` function were changed from synchronous to asynchronous to support the new async signature of `resolve_node_dir`, which now requires a capability reference for agent workspace resolution. This ensures that working directory resolution properly awaits the underlying filesystem checks and capability validation.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/nodes/integration/agent.rs                |  2 +-
 src/nodes/integration/agent_request.rs        | 10 ++++++++--
 .../integration/sub_workflow/execution.rs     | 20 +++++++++++--------
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/nodes/integration/agent.rs b/src/nodes/integration/agent.rs
index 33386d1..94ce06e 100644
--- a/src/nodes/integration/agent.rs
+++ b/src/nodes/integration/agent.rs
@@ -189,7 +189,7 @@ async fn run_turn_indexed(
         } else {
             "working_dir"
         };
-        let resolved = super::agent_request::resolve_working_dir(ctx, &raw, key)?;
+        let resolved = super::agent_request::resolve_working_dir(ctx, &raw, key).await?;
         if let Value::Object(map) = &mut request {
             map.insert("cwd".to_string(), Value::from(resolved.clone()));
             map.insert("working_dir".to_string(), Value::from(resolved));
diff --git a/src/nodes/integration/agent_request.rs b/src/nodes/integration/agent_request.rs
index d6427f5..6f09aa7 100644
--- a/src/nodes/integration/agent_request.rs
+++ b/src/nodes/integration/agent_request.rs
@@ -240,13 +240,19 @@ pub(crate) fn declared_working_dir(cfg: &Value, node_id: &str) -> Result<Option<
 /// # Errors
 /// Returns [`EngineError::Capability`] when the directory escapes the
 /// workspace, does not exist, or is not a directory.
-pub(crate) fn resolve_working_dir(ctx: &NodeContext<'_>, raw: &str, key: &str) -> Result<String> {
+pub(crate) async fn resolve_working_dir(
+    ctx: &NodeContext<'_>,
+    raw: &str,
+    key: &str,
+) -> Result<String> {
     crate::workdir::resolve_node_dir(
+        ctx.caps.agent.as_ref(),
         ctx.run,
         raw,
         &format!("config.{key}"),
         &format!("agent node {}", ctx.node.id),
     )
+    .await
 }
 
 /// Resolves each declared [`ContextSource`] into a [`ContextBlock`], in
@@ -415,7 +421,7 @@ pub(crate) async fn assemble(
         } else {
             "working_dir"
         };
-        agent.working_dir = Some(resolve_working_dir(ctx, &raw, key)?);
+        agent.working_dir = Some(resolve_working_dir(ctx, &raw, key).await?);
     }
     let identity = identity_of(ctx, item_index);
     let conn = cfg.get("connection_ref").and_then(Value::as_str);
diff --git a/src/nodes/integration/sub_workflow/execution.rs b/src/nodes/integration/sub_workflow/execution.rs
index 72cc5e5..605b891 100644
--- a/src/nodes/integration/sub_workflow/execution.rs
+++ b/src/nodes/integration/sub_workflow/execution.rs
@@ -126,7 +126,7 @@ fn pause_for_child_gates(node_id: &str, gates: Vec<String>) -> NodeOutput {
 /// (see [`crate::workdir`]). A parent run with no workspace has nothing to
 /// contain the value against, so it is taken as written: that is how a graph
 /// declares a workspace for a child when the run itself was never pinned to one.
-fn child_workspace(ctx: &NodeContext<'_>, scope: &Value) -> Result<Option<String>> {
+async fn child_workspace(ctx: &NodeContext<'_>, scope: &Value) -> Result<Option<String>> {
     let Some(declared) = ctx.node.config.get("workspace").filter(|v| !v.is_null()) else {
         return Ok(crate::workdir::run_workspace(ctx.run).map(str::to_string));
     };
@@ -143,12 +143,16 @@ fn child_workspace(ctx: &NodeContext<'_>, scope: &Value) -> Result<Option<String
             ctx.node.id
         )));
     }
-    Ok(Some(crate::workdir::resolve_node_dir(
-        ctx.run,
-        raw,
-        "config.workspace",
-        &format!("sub_workflow node {}", ctx.node.id),
-    )?))
+    Ok(Some(
+        crate::workdir::resolve_node_dir(
+            ctx.caps.agent.as_ref(),
+            ctx.run,
+            raw,
+            "config.workspace",
+            &format!("sub_workflow node {}", ctx.node.id),
+        )
+        .await?,
+    ))
 }
 
 /// What one child run produced, from the parent node's point of view.
@@ -259,7 +263,7 @@ async fn run_child(
     // the child run's workspace, held to the same containment rule as an
     // `agent` node's `cwd`: it must resolve inside the parent's workspace. With
     // no override the child inherits the parent's.
-    let child_workspace = child_workspace(ctx, scope)?;
+    let child_workspace = child_workspace(ctx, scope).await?;
     // Box the recursive engine call so the async future type stays sized.
     // Forward the parent run's cancellation token: cancelling the parent must
     // wind down this child too, rather than letting it run on orphaned behind a

From 20c4cdfa851e101e363fedcfb794981e2646da81 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:37:07 +0300
Subject: [PATCH 09/11] test(workdir): add harness tests for remote workspace
 resolution

Add test coverage for the new `AgentRunner` capability that allows remote hosts to resolve workspace directories without touching the local filesystem. The tests verify that a harness can answer for its own workspace, that refusals fail with the correct reason, that unmanaged answers fall back to local disk, and that path traversal checks still run before the harness is consulted.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/workdir_tests.rs | 134 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 127 insertions(+), 7 deletions(-)

diff --git a/src/workdir_tests.rs b/src/workdir_tests.rs
index f03ab18..89420c2 100644
--- a/src/workdir_tests.rs
+++ b/src/workdir_tests.rs
@@ -9,6 +9,7 @@ use serde_json::json;
 use super::{
     Absolute, resolve_dir_in_workspace, resolve_in_workspace, resolve_node_dir, run_workspace,
 };
+use crate::caps::WorkdirCheck;
 
 /// A workspace with a `worktrees/issue-1` directory and a file in it.
 fn workspace() -> tempfile::TempDir {
@@ -173,22 +174,141 @@ fn the_run_workspace_comes_from_the_run_slice_then_the_trigger() {
     assert_eq!(run_workspace(&json!({})), None);
 }
 
-#[test]
-fn a_run_with_no_workspace_passes_the_directory_through() {
+#[tokio::test]
+async fn a_run_with_no_workspace_passes_the_directory_through() {
     // A harness whose agents run in a remote sandbox names directories this
     // process has never heard of; checking them locally would fail every one.
-    let resolved = resolve_node_dir(&json!({}), "/srv/checkout", "config.cwd", "agent node a")
-        .expect("no workspace, no resolution");
+    let resolved = resolve_node_dir(
+        None,
+        &json!({}),
+        "/srv/checkout",
+        "config.cwd",
+        "agent node a",
+    )
+    .await
+    .expect("no workspace, no resolution");
 
     assert_eq!(resolved, "/srv/checkout");
 }
 
-#[test]
-fn a_resolved_directory_is_reported_with_the_node_surface() {
+#[tokio::test]
+async fn a_resolved_directory_is_reported_with_the_node_surface() {
     let root = workspace();
     let run = json!({ "workspace": root.path().to_string_lossy() });
-    let error = resolve_node_dir(&run, "nope", "config.cwd", "agent node prepare")
+    let error = resolve_node_dir(None, &run, "nope", "config.cwd", "agent node prepare")
+        .await
         .expect_err("a missing directory fails the step");
 
     assert!(error.to_string().contains("agent node prepare:"), "{error}");
 }
+
+/// An [`AgentRunner`] whose agents run on a filesystem this process cannot see:
+/// it answers for its own workspace and never touches the local disk.
+struct RemoteHarness {
+    answer: WorkdirCheck,
+}
+
+#[async_trait::async_trait]
+impl crate::caps::AgentRunner for RemoteHarness {
+    async fn run_agent(
+        &self,
+        _agent_ref: &str,
+        _request: serde_json::Value,
+        _conn: Option<&str>,
+    ) -> crate::error::Result<serde_json::Value> {
+        Ok(json!({}))
+    }
+
+    async fn resolve_workdir(&self, _workspace: &str, _declared: &str) -> WorkdirCheck {
+        self.answer.clone()
+    }
+}
+
+fn harness(answer: WorkdirCheck) -> std::sync::Arc<dyn crate::caps::AgentRunner> {
+    std::sync::Arc::new(RemoteHarness { answer })
+}
+
+#[tokio::test]
+async fn a_harness_that_owns_the_workspace_answers_for_it() {
+    // The workspace and the directory are both remote: nothing here exists on
+    // this process's disk, and the local check would have refused both.
+    let runner = harness(WorkdirCheck::Resolved("/remote/ws/worktrees/1".to_string()));
+    let run = json!({ "workspace": "/remote/ws" });
+
+    let resolved = resolve_node_dir(
+        Some(&runner),
+        &run,
+        "worktrees/1",
+        "config.cwd",
+        "agent node code",
+    )
+    .await
+    .expect("the harness resolved it");
+
+    assert_eq!(resolved, "/remote/ws/worktrees/1");
+}
+
+#[tokio::test]
+async fn a_harness_refusal_fails_the_step_with_its_reason() {
+    let runner = harness(WorkdirCheck::Refused("is not in the sandbox".to_string()));
+    let run = json!({ "workspace": "/remote/ws" });
+
+    let error = resolve_node_dir(
+        Some(&runner),
+        &run,
+        "worktrees/1",
+        "config.cwd",
+        "agent node code",
+    )
+    .await
+    .expect_err("a refusal fails the step");
+
+    let error = error.to_string();
+    assert!(error.contains("agent node code:"), "{error}");
+    assert!(error.contains("is not in the sandbox"), "{error}");
+    assert!(error.contains("worktrees/1"), "{error}");
+}
+
+#[tokio::test]
+async fn an_unmanaged_answer_falls_back_to_the_local_filesystem() {
+    // The default for every existing host: the engine checks its own disk,
+    // exactly as it did before the capability existed.
+    let runner = harness(WorkdirCheck::Unmanaged);
+    let root = workspace();
+    let run = json!({ "workspace": root.path().to_string_lossy() });
+
+    let resolved = resolve_node_dir(
+        Some(&runner),
+        &run,
+        "worktrees/issue-1",
+        "config.cwd",
+        "agent node code",
+    )
+    .await
+    .expect("the local filesystem answers");
+
+    assert!(resolved.ends_with("worktrees/issue-1"), "{resolved}");
+}
+
+#[tokio::test]
+async fn the_shape_check_runs_before_the_harness_is_consulted() {
+    // A host cannot be asked to bless a `..` escape: the syntactic half is the
+    // engine's, on every filesystem.
+    let runner = harness(WorkdirCheck::Resolved("/anywhere".to_string()));
+    let run = json!({ "workspace": "/remote/ws" });
+
+    let error = resolve_node_dir(
+        Some(&runner),
+        &run,
+        "../../etc",
+        "config.cwd",
+        "agent node code",
+    )
+    .await
+    .expect_err("traversal is refused before the harness sees it");
+
+    assert!(
+        error.to_string().contains("must not traverse outside"),
+        "{error}"
+    );
+}

From 90f2aa095f2d612f631e763e39b0c9e3d7b08127 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:37:58 +0300
Subject: [PATCH 10/11] feat(wiki): document null expression and filesystem
 semantics for workdir resolution

Added two new paragraphs to the Node-Catalog page that clarify how the engine handles workdir resolution. The first explains that an expression resolving to null fails the step rather than silently falling back to a default directory, preventing steps from running in an unintended checkout. The second describes how the engine checks the shape of a declared directory but delegates filesystem existence checks to the agent runner, with the shell node taking a separate path through the shell runner and script policy.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 wiki/Node-Catalog.md | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/wiki/Node-Catalog.md b/wiki/Node-Catalog.md
index 7ec1703..63571ec 100644
--- a/wiki/Node-Catalog.md
+++ b/wiki/Node-Catalog.md
@@ -97,10 +97,26 @@ falling back to the workspace. Every one of them is `=`-bindable, which is the
 point — `"cwd": "=nodes.prepare.item.json.worktree"` runs the step in a
 directory an earlier node created.
 
+An expression that resolves to `null` — the upstream node failed, or the key
+moved — **fails the step** too. It is not read as "no directory declared": that
+would fall back to the agent definition's own `working_dir`, then to whatever
+the harness defaults to, and the step would run in a different checkout without
+saying so.
+
 A run with **no** workspace resolves nothing: the string reaches the harness
 verbatim, as it always has, because a host whose agents run in a remote sandbox
 names directories this process has never heard of.
 
+**Whose filesystem.** The shape of a declared directory — absolute vs relative,
+`..` traversal — is decided by reading the string, so the engine always checks
+it. Whether the path *exists*, what it canonicalizes to, and whether it is a
+directory are outside-world effects, and they route through
+`AgentRunner::resolve_workdir`. A harness whose agents run in a container or a
+remote sandbox implements it and answers for its own filesystem; the default is
+`WorkdirCheck::Unmanaged`, which checks the engine's own disk exactly as before.
+The `shell` node reaches the same place by a different road: it hands `cwd` to
+the `ShellRunner` untouched and the host's `ScriptPolicy` contains it.
+
 A directory key a node does not read — `workdir` on an `agent` node, `cwd` on a
 `tool_call` node — is a **validation error**, not a silent no-op. Being able to
 write down where a step runs and have it ignored is the failure this whole seam

From 74b8cfca9104459dc190a4591c17d3e498d40c27 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Sun, 23 Aug 2026 15:38:31 +0300
Subject: [PATCH 11/11] chore(changelog): update entry for workdir resolution
 change

Expanded the changelog entry for the workdir resolution change to clarify that a null expression now fails the step, and added a new entry documenting the `AgentRunner::resolve_workdir` and `caps::WorkdirCheck` interface for harnesses that need to answer for their own filesystem.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 CHANGELOG.md | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d4d344c..fccb65b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,8 +46,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   hoisted into a shared module so there is exactly one of them: relative paths
   join the workspace, absolute paths must resolve inside it, symlinks are
   followed, and a missing path or a non-directory fails the step instead of
-  falling back to the workspace. A run with no workspace resolves nothing and
-  passes the value to the harness verbatim, as before.
+  falling back to the workspace. An expression that resolves to `null` fails the
+  step too, rather than reading as "no directory declared" and falling through
+  to the agent definition's own `working_dir` or the harness default. A run with
+  no workspace resolves nothing and passes the value to the harness verbatim, as
+  before.
+
+- **`AgentRunner::resolve_workdir` + `caps::WorkdirCheck`** — the seam a harness
+  uses to answer for its **own** filesystem when a run does declare a workspace.
+  Deciding whether a directory exists, what it canonicalizes to, and whether it
+  is a directory is an outside-world effect, and on a harness whose agents run
+  in a container or a remote sandbox the answer is not on the engine's disk.
+  The shape of the value — absolute vs relative, `..` traversal — is string
+  arithmetic and stays with the engine, so a host cannot weaken the containment
+  check that matters most. The method has a default returning
+  `WorkdirCheck::Unmanaged`, which checks the engine's own filesystem exactly as
+  before, so every existing `AgentRunner` keeps compiling and behaving
+  identically.
 
 ### Changed