From 74d5874384f403d15c8bcf27c307e7e792a5fadc Mon Sep 17 00:00:00 2001
From: MechaCat02 <fabian@diekaulbachs.de>
Date: Sun, 12 Jul 2026 17:30:16 +0200
Subject: [PATCH] =?UTF-8?q?feat(workflows):=20M4=20=E2=80=94=20nested=20su?=
 =?UTF-8?q?b-workflows=20(start=5Fchild,=20parent-park,=20depth=20ceiling)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A `workflow`-kind step now starts a nested sub-workflow run instead of a
function. The orchestrator's step processing is restructured into prepare →
dispatch (`Prep`): after the shared context/`when`/input resolution, a
function step runs through the executor as before, while a workflow step:

- checks the nesting depth ceiling (`PICLOUD_WORKFLOW_MAX_DEPTH`; the child
  would run at parent depth + 1) — over-deep → the step fails, not infinite;
- resolves the child workflow by name in the run's app scope;
- `start_child_and_park`: in one token-gated tx, seeds a child run (depth + 1,
  `parent_run_id`/`parent_step_id` linkage, correlated under the same
  `root_execution_id`) and PARKS the parent step (`running`, claim_token
  cleared, `child_run_id` set). A parked step is never re-claimed (only `ready`
  steps are) nor reclaimed (only *leased* running steps). The token-gated park
  runs before the child insert, so a stale claim writes nothing (no orphan).

Each tick first runs `resume_finished_children`: a parent step parked on a
now-terminal child is resolved (child output → parent step output, or child
error → parent step failed) and the parent run advanced — idempotent via a
`status='running'` gate. The child's own steps are claimed by the same global
scan, so nesting is just more runs. A failed sub-workflow honors the parent
step's `on_error`.

Shared plumbing extracted for reuse: `advance_run_tx` (out of
`complete_step_and_advance`) and `seed_run_tx` (out of `start_run`).

Apply-time soft-warn (`workflow_nesting_warnings`, wired into `plan_warnings`):
a workflow that nests into itself — directly or via a mutual cycle within the
bundle — is flagged at plan (bounded by the depth ceiling, but almost always a
bug). Not an error.

Tests: nested output-flows-to-parent + depth-ceiling-fails-the-run (DB-gated,
end-to-end), self/mutual-cycle warning (pure unit). fmt + clippy -D warnings
clean, 449 manager-core lib tests, 13 orchestrator DB tests, 26 CLI journeys
(workflows/apply/plan/prune) green, binary boots.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 crates/manager-core/src/apply_service.rs      | 102 +++++++
 .../manager-core/src/workflow_orchestrator.rs | 279 +++++++++++++-----
 crates/manager-core/src/workflow_repo.rs      | 229 ++++++++++++--
 .../tests/workflow_orchestrator.rs            | 173 ++++++++++-
 4 files changed, 674 insertions(+), 109 deletions(-)

diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs
index 8bea2a5..a8562c0 100644
--- a/crates/manager-core/src/apply_service.rs
+++ b/crates/manager-core/src/apply_service.rs
@@ -3616,6 +3616,7 @@ impl ApplyService {
             warnings.extend(unreachable_endpoint_warnings(bundle));
         }
         warnings.extend(self.dangling_suppress_warnings(owner, bundle).await?);
+        warnings.extend(workflow_nesting_warnings(bundle));
         Ok(warnings)
     }
 
@@ -5280,6 +5281,64 @@ fn disabled_target_warnings(bundle: &Bundle) -> Vec<String> {
     out
 }
 
+/// M4 soft-warn: a `workflow`-kind step that (directly or transitively, within
+/// this bundle) nests back into its own workflow forms a cycle. This is not an
+/// error — the runtime depth ceiling (`PICLOUD_WORKFLOW_MAX_DEPTH`) bounds it —
+/// but it's almost always a mistake, so we surface it at plan time. Only edges
+/// to workflows present in the same bundle are considered (a reference to an
+/// external workflow can't be resolved here).
+fn workflow_nesting_warnings(bundle: &Bundle) -> Vec<String> {
+    use std::collections::{BTreeSet, HashMap, HashSet};
+    if bundle.workflows.is_empty() {
+        return Vec::new();
+    }
+    let names: HashSet<String> = bundle
+        .workflows
+        .iter()
+        .map(|w| w.name.to_lowercase())
+        .collect();
+    // from → the set of in-bundle workflows it nests into.
+    let mut edges: HashMap<String, BTreeSet<String>> = HashMap::new();
+    for w in &bundle.workflows {
+        let from = w.name.to_lowercase();
+        for s in &w.definition.steps {
+            if let Some(t) = s.workflow.as_deref() {
+                let t = t.to_lowercase();
+                if names.contains(&t) {
+                    edges.entry(from.clone()).or_default().insert(t);
+                }
+            }
+        }
+    }
+    let mut out = Vec::new();
+    for w in &bundle.workflows {
+        let start = w.name.to_lowercase();
+        // Reachability from `start`: if we can get back to `start`, it's cyclic.
+        let mut seen: HashSet<String> = HashSet::new();
+        let mut stack: Vec<String> = edges.get(&start).into_iter().flatten().cloned().collect();
+        let mut cyclic = false;
+        while let Some(n) = stack.pop() {
+            if n == start {
+                cyclic = true;
+                break;
+            }
+            if seen.insert(n.clone()) {
+                if let Some(next) = edges.get(&n) {
+                    stack.extend(next.iter().cloned());
+                }
+            }
+        }
+        if cyclic {
+            out.push(format!(
+                "workflow `{}` can nest into itself (a sub-workflow cycle) — runs are \
+                 bounded by PICLOUD_WORKFLOW_MAX_DEPTH, not infinite, but this is likely a bug",
+                w.name
+            ));
+        }
+    }
+    out
+}
+
 /// Per-kind structural validation for one desired trigger — kept at parity
 /// with the interactive trigger API so `apply` can't write a trigger the
 /// dashboard would have rejected. Pure (no live state), so it's unit-tested
@@ -6695,4 +6754,47 @@ mod tests {
         ]));
         assert!(validate_workflow_definition("w", &bad_tmpl).is_err());
     }
+
+    fn bundle_workflow(name: &str, steps: serde_json::Value) -> BundleWorkflow {
+        BundleWorkflow {
+            name: name.into(),
+            definition: wf_def(steps),
+            enabled: true,
+        }
+    }
+
+    #[test]
+    fn workflow_nesting_warning_flags_self_and_mutual_cycles() {
+        // Direct self-nest.
+        let mut b = empty_bundle();
+        b.workflows = vec![bundle_workflow(
+            "loop",
+            serde_json::json!([{ "name": "s", "workflow": "loop" }]),
+        )];
+        let w = workflow_nesting_warnings(&b);
+        assert_eq!(w.len(), 1);
+        assert!(w[0].contains("loop") && w[0].contains("cycle"));
+
+        // Mutual cycle w1 -> w2 -> w1.
+        let mut b = empty_bundle();
+        b.workflows = vec![
+            bundle_workflow("w1", serde_json::json!([{ "name": "s", "workflow": "w2" }])),
+            bundle_workflow("w2", serde_json::json!([{ "name": "s", "workflow": "w1" }])),
+        ];
+        assert_eq!(workflow_nesting_warnings(&b).len(), 2);
+
+        // Acyclic nesting (parent -> child, child is a leaf) → no warning.
+        let mut b = empty_bundle();
+        b.workflows = vec![
+            bundle_workflow(
+                "parent",
+                serde_json::json!([{ "name": "s", "workflow": "child" }]),
+            ),
+            bundle_workflow(
+                "child",
+                serde_json::json!([{ "name": "s", "function": "f" }]),
+            ),
+        ];
+        assert!(workflow_nesting_warnings(&b).is_empty());
+    }
 }
diff --git a/crates/manager-core/src/workflow_orchestrator.rs b/crates/manager-core/src/workflow_orchestrator.rs
index 41e80e9..db760a9 100644
--- a/crates/manager-core/src/workflow_orchestrator.rs
+++ b/crates/manager-core/src/workflow_orchestrator.rs
@@ -27,9 +27,13 @@
 //! A second, slower cadence reclaims steps leased by a crashed worker
 //! ([`workflow_repo::reclaim_stale_steps`]) — the durability safety net.
 //!
-//! M2 executes **function** steps only; `workflow`-kind (nested sub-workflow)
-//! steps land in M4, and `when` conditions + input templating in M3. The seams
-//! are present (the step target enum, `run_input`, `workflow_depth`).
+//! Each claimed step is *prepared* ([`Prep`]) — the run context is built, its
+//! `when` condition evaluated (false → `skipped`, M3), and its `input` template
+//! resolved (M3) — then dispatched by target: a **function** runs through the
+//! executor; a **`workflow`** starts a nested child run and parks the parent
+//! step on it ([`workflow_repo::start_child_and_park`], M4), resumed by
+//! [`workflow_repo::resume_finished_children`] once the child terminates. Nested
+//! runs are bounded by [`WorkflowOrchestratorConfig::max_depth`].
 //!
 //! [`cron_scheduler`]: crate::cron_scheduler
 
@@ -42,19 +46,41 @@ use tokio::sync::OwnedSemaphorePermit;
 
 use picloud_executor_core::{build_execution_log, ExecRequest, InvocationType};
 use picloud_orchestrator_core::{ExecutionGate, ExecutorClient, ScriptIdentity};
-use picloud_shared::workflow::{StepTarget, WorkflowBackoff, WorkflowStepDef};
-use picloud_shared::{ExecutionId, ExecutionLogSink, ExecutionSource, RequestId};
+use picloud_shared::workflow::{StepTarget, WorkflowBackoff, WorkflowDefinition, WorkflowStepDef};
+use picloud_shared::{
+    ExecutionId, ExecutionLogSink, ExecutionSource, RequestId, Script, WorkflowId,
+};
 
 use crate::dispatcher::compute_backoff;
 use crate::repo::ScriptRepository;
 use crate::trigger_config::BackoffShape;
 use crate::workflow_expr;
 use crate::workflow_repo::{
-    claim_ready_step, complete_step_and_advance, load_run_step_outputs, reclaim_stale_steps,
-    ClaimedStep, StepOutcome,
+    claim_ready_step, complete_step_and_advance, get_workflow_by_name, load_run_step_outputs,
+    reclaim_stale_steps, resume_finished_children, start_child_and_park, ClaimedStep, StepOutcome,
 };
 use crate::workflow_template::{self, RunContext};
 
+/// The prepared action for a claimed step, after context build + `when` + input
+/// resolution + target lookup. The orchestrator dispatches on this.
+enum Prep {
+    /// `when` evaluated false — skip the step (never executed).
+    Skip,
+    /// A resolution error, already turned into a `Failed` outcome.
+    Fail(StepOutcome),
+    /// A function step: run this script with this resolved body.
+    Function {
+        script: Box<Script>,
+        body: serde_json::Value,
+    },
+    /// A nested sub-workflow step: start this child run with this input.
+    StartChild {
+        child_id: WorkflowId,
+        child_def: WorkflowDefinition,
+        input: serde_json::Value,
+    },
+}
+
 /// Max steps claimed (and thus gate permits held) per orchestrator tick. Keeps
 /// the workflow worker from monopolizing the gate it shares with HTTP + the
 /// dispatcher; the tick cadence + reclaim pick up the rest.
@@ -182,6 +208,15 @@ impl WorkflowOrchestrator {
     pub async fn tick_once(&self) {
         use futures::stream::{self, StreamExt};
 
+        // M4: resolve any parent steps whose child sub-workflow has finished —
+        // first, so a just-completed child promotes the parent's dependents to
+        // `ready` in time for this tick's claim scan below.
+        match resume_finished_children(&self.pool).await {
+            Ok(0) => {}
+            Ok(n) => tracing::debug!(resolved = n, "workflow parent steps resumed"),
+            Err(e) => tracing::warn!(?e, "workflow child-resume errored"),
+        }
+
         let mut batch: Vec<(ClaimedStep, OwnedSemaphorePermit)> = Vec::new();
         for _ in 0..STEP_CLAIM_BATCH {
             // Acquire the gate slot BEFORE claiming so a claimed step always
@@ -213,12 +248,50 @@ impl WorkflowOrchestrator {
             .await;
     }
 
-    /// Execute one claimed step, then write its outcome + advance the run.
+    /// Process one claimed step: prepare it (context → `when` → input →
+    /// target), then dispatch — run a function, start a sub-workflow, skip, or
+    /// fail.
     async fn run_step(&self, claimed: ClaimedStep, permit: OwnedSemaphorePermit) {
-        let outcome = self.execute_step(&claimed).await;
-        // Release the gate slot before the (DB-only, fast) advance write.
-        drop(permit);
-        if let Err(e) = complete_step_and_advance(&self.pool, &claimed, outcome).await {
+        match self.prepare(&claimed).await {
+            Prep::Skip => {
+                drop(permit);
+                self.complete(&claimed, StepOutcome::Skipped).await;
+            }
+            Prep::Fail(outcome) => {
+                drop(permit);
+                self.complete(&claimed, outcome).await;
+            }
+            Prep::Function { script, body } => {
+                let outcome = self.execute_function(&claimed, &script, body).await;
+                // Release the gate slot before the (DB-only, fast) advance.
+                drop(permit);
+                self.complete(&claimed, outcome).await;
+            }
+            Prep::StartChild {
+                child_id,
+                child_def,
+                input,
+            } => {
+                // A nested start runs no script — release the gate slot; the
+                // child's own steps compete for it when they run.
+                drop(permit);
+                if let Err(e) =
+                    start_child_and_park(&self.pool, &claimed, child_id, &child_def, input).await
+                {
+                    tracing::error!(
+                        ?e,
+                        run_id = %claimed.run_id,
+                        step = %claimed.step_name,
+                        "workflow nested-start write failed"
+                    );
+                }
+            }
+        }
+    }
+
+    /// Write a step outcome + advance the run, logging any DB error.
+    async fn complete(&self, claimed: &ClaimedStep, outcome: StepOutcome) {
+        if let Err(e) = complete_step_and_advance(&self.pool, claimed, outcome).await {
             tracing::error!(
                 ?e,
                 run_id = %claimed.run_id,
@@ -228,17 +301,17 @@ impl WorkflowOrchestrator {
         }
     }
 
-    /// Resolve + run a step's function, returning the outcome. Failures are
-    /// converted to [`StepOutcome::Failed`] carrying the retry delay derived
-    /// from the step's own retry policy (the repo decides retry-vs-terminal).
+    /// Prepare a claimed step: build the run context, evaluate `when`, resolve
+    /// the input template, and resolve the target. Pure of side effects beyond
+    /// the reads it needs; the caller dispatches on the returned [`Prep`].
     #[allow(clippy::too_many_lines)]
-    async fn execute_step(&self, claimed: &ClaimedStep) -> StepOutcome {
+    async fn prepare(&self, claimed: &ClaimedStep) -> Prep {
         let Some(step) = claimed.step_def() else {
-            return self.fail(
+            return Prep::Fail(self.fail(
                 None,
                 claimed,
                 "step not found in workflow definition".into(),
-            );
+            ));
         };
 
         // M3: build the run context (run input + prior succeeded outputs) once;
@@ -246,7 +319,11 @@ impl WorkflowOrchestrator {
         let outputs = match load_run_step_outputs(&self.pool, claimed.run_id).await {
             Ok(o) => o,
             Err(e) => {
-                return self.fail(Some(step), claimed, format!("loading step context: {e}"));
+                return Prep::Fail(self.fail(
+                    Some(step),
+                    claimed,
+                    format!("loading step context: {e}"),
+                ));
             }
         };
         let ctx = RunContext {
@@ -254,84 +331,125 @@ impl WorkflowOrchestrator {
             steps: outputs,
         };
 
-        // M3: a false `when` condition skips the step — never executed, and
-        // satisfied-but-empty for its dependents (`compute_advance`).
+        // M3: a false `when` skips the step — never executed, satisfied-but-empty.
         if let Some(cond) = &step.when {
             match workflow_expr::eval(cond, &ctx) {
                 Ok(true) => {}
-                Ok(false) => return StepOutcome::Skipped,
+                Ok(false) => return Prep::Skip,
                 Err(e) => {
-                    return self.fail(Some(step), claimed, format!("evaluating `when`: {e}"));
+                    return Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("evaluating `when`: {e}"),
+                    ));
                 }
             }
         }
 
-        let fn_name = match step.target() {
-            Some(StepTarget::Function(f)) => f.to_string(),
-            Some(StepTarget::Workflow(_)) => {
-                // Nested sub-workflows land in M4.
-                return self.fail(
-                    Some(step),
-                    claimed,
-                    "sub-workflow steps not yet supported".into(),
-                );
-            }
-            None => {
-                return self.fail(
-                    Some(step),
-                    claimed,
-                    "step declares neither `function` nor `workflow`".into(),
-                );
-            }
-        };
-
-        // Resolve the function by name in the RUN's app scope (an inherited
-        // group script is reachable, nearest-owner-wins) — never off a
-        // script-passed arg. cx isolation boundary preserved.
-        let script = match self
-            .scripts
-            .get_by_name_inherited(claimed.app_id, &fn_name)
-            .await
-        {
-            Ok(Some(s)) if s.enabled => s,
-            Ok(Some(_)) => {
-                return self.fail(
-                    Some(step),
-                    claimed,
-                    format!("function {fn_name:?} is disabled"),
-                );
-            }
-            Ok(None) => {
-                return self.fail(
-                    Some(step),
-                    claimed,
-                    format!("function {fn_name:?} not found"),
-                );
-            }
-            Err(e) => {
-                return self.fail(Some(step), claimed, format!("resolving {fn_name:?}: {e}"));
-            }
-        };
-
-        // M3: resolve `{{ … }}` references in the step input against the
-        // context. A null input passes the run input through unchanged; a
-        // missing reference is a hard step failure (a definition bug we
-        // surface, not hide).
+        // M3: resolve `{{ … }}` references in the step input. Null passes the
+        // run input through; a missing reference is a hard failure.
         let body = if step.input.is_null() {
             claimed.run_input.clone()
         } else {
             match workflow_template::resolve(&step.input, &ctx) {
                 Ok(v) => v,
                 Err(e) => {
-                    return self.fail(Some(step), claimed, format!("resolving input: {e}"));
+                    return Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("resolving input: {e}"),
+                    ));
                 }
             }
         };
 
-        let execution_id = ExecutionId::new();
+        match step.target() {
+            Some(StepTarget::Function(fn_name)) => {
+                // Resolve the function by name in the RUN's app scope (an
+                // inherited group script is reachable, nearest-owner-wins) —
+                // never off a script-passed arg. cx isolation boundary held.
+                match self
+                    .scripts
+                    .get_by_name_inherited(claimed.app_id, fn_name)
+                    .await
+                {
+                    Ok(Some(s)) if s.enabled => Prep::Function {
+                        script: Box::new(s),
+                        body,
+                    },
+                    Ok(Some(_)) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("function {fn_name:?} is disabled"),
+                    )),
+                    Ok(None) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("function {fn_name:?} not found"),
+                    )),
+                    Err(e) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("resolving {fn_name:?}: {e}"),
+                    )),
+                }
+            }
+            Some(StepTarget::Workflow(wf_name)) => {
+                // M4: nesting depth ceiling — the child would run at depth + 1.
+                let ceiling = i32::try_from(self.config.max_depth).unwrap_or(i32::MAX);
+                if claimed.workflow_depth + 1 > ceiling {
+                    return Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!(
+                            "max workflow nesting depth ({}) exceeded",
+                            self.config.max_depth
+                        ),
+                    ));
+                }
+                match get_workflow_by_name(&self.pool, claimed.app_id, wf_name).await {
+                    Ok(Some(w)) if w.enabled => Prep::StartChild {
+                        child_id: w.id,
+                        child_def: w.definition,
+                        input: body,
+                    },
+                    Ok(Some(_)) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("workflow {wf_name:?} is disabled"),
+                    )),
+                    Ok(None) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("workflow {wf_name:?} not found"),
+                    )),
+                    Err(e) => Prep::Fail(self.fail(
+                        Some(step),
+                        claimed,
+                        format!("resolving workflow {wf_name:?}: {e}"),
+                    )),
+                }
+            }
+            None => Prep::Fail(self.fail(
+                Some(step),
+                claimed,
+                "step declares neither `function` nor `workflow`".into(),
+            )),
+        }
+    }
+
+    /// Run a resolved function step through the executor, returning its outcome
+    /// (a failure carries the step's retry-policy backoff). Logs the run under
+    /// `ExecutionSource::Workflow` so `pic logs` surfaces it.
+    async fn execute_function(
+        &self,
+        claimed: &ClaimedStep,
+        script: &Script,
+        body: serde_json::Value,
+    ) -> StepOutcome {
         let request_id = RequestId::new();
         let req = ExecRequest {
-            execution_id,
+            execution_id: ExecutionId::new(),
             request_id,
             script_id: script.id,
             script_name: script.name.clone(),
@@ -346,8 +464,8 @@ impl WorkflowOrchestrator {
             sandbox_overrides: script.sandbox,
             app_id: claimed.app_id,
             script_owner: script.owner(),
-            // Like invoke_async / HTTP outbox rows: steps run with no
-            // principal (the run's origin is forensic, not re-authenticated).
+            // Like invoke_async / HTTP outbox rows: steps run with no principal
+            // (the run's origin is forensic, not re-authenticated).
             principal: None,
             trigger_depth: 0,
             root_execution_id: ExecutionId::from(claimed.root_execution_id),
@@ -366,7 +484,6 @@ impl WorkflowOrchestrator {
             .await;
         let finished = Utc::now();
 
-        // Log the run so `pic logs` surfaces workflow steps (source=workflow).
         let log = build_execution_log(
             claimed.app_id,
             script.id,
@@ -385,7 +502,7 @@ impl WorkflowOrchestrator {
 
         match outcome {
             Ok(resp) => StepOutcome::Succeeded(resp.body),
-            Err(err) => self.fail(Some(step), claimed, err.to_string()),
+            Err(err) => self.fail(claimed.step_def(), claimed, err.to_string()),
         }
     }
 
diff --git a/crates/manager-core/src/workflow_repo.rs b/crates/manager-core/src/workflow_repo.rs
index bb51878..6723fab 100644
--- a/crates/manager-core/src/workflow_repo.rs
+++ b/crates/manager-core/src/workflow_repo.rs
@@ -461,6 +461,19 @@ pub async fn start_run(
     definition: &WorkflowDefinition,
 ) -> Result<WorkflowRunId, WorkflowRepoError> {
     let mut tx = pool.begin().await?;
+    let run_id = seed_run_tx(&mut tx, &new, definition).await?;
+    tx.commit().await?;
+    Ok(run_id)
+}
+
+/// Insert the `workflow_runs` row + its `workflow_run_steps` (roots `ready`,
+/// the rest `pending`) inside an open transaction. Shared by `start_run` and
+/// the nested-child start (`start_child_and_park`).
+pub async fn seed_run_tx(
+    tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
+    new: &NewWorkflowRun,
+    definition: &WorkflowDefinition,
+) -> Result<WorkflowRunId, WorkflowRepoError> {
     let (run_id,): (Uuid,) = sqlx::query_as(
         "INSERT INTO workflow_runs \
             (workflow_id, app_id, status, input, root_execution_id, \
@@ -474,7 +487,7 @@ pub async fn start_run(
     .bind(new.workflow_depth)
     .bind(new.parent_run_id.map(WorkflowRunId::into_inner))
     .bind(new.parent_step_id.map(WorkflowRunStepId::into_inner))
-    .fetch_one(&mut *tx)
+    .fetch_one(&mut **tx)
     .await?;
 
     for step in &definition.steps {
@@ -493,11 +506,10 @@ pub async fn start_run(
         .bind(&step.name)
         .bind(status)
         .bind(max_attempts)
-        .execute(&mut *tx)
+        .execute(&mut **tx)
         .await?;
     }
 
-    tx.commit().await?;
     Ok(run_id.into())
 }
 
@@ -683,28 +695,43 @@ pub async fn complete_step_and_advance(
         }
     }
 
-    // 2. Advance — lock the run row so concurrent advances of the same run
-    // serialize (each does a full recompute, so this is idempotent).
+    // 2. Advance the run graph in the same tx.
+    advance_run_tx(&mut tx, claimed.run_id, &claimed.definition).await?;
+    tx.commit().await?;
+    Ok(AdvanceResult::Advanced)
+}
+
+/// Advance one run's graph inside an open transaction: lock the run row (so
+/// concurrent advances serialize — each does a full recompute, so this is
+/// idempotent), promote pending steps whose deps are now satisfied, and write
+/// the run's terminal status. A no-op if the run is already terminal. Does NOT
+/// commit — the caller owns the transaction.
+///
+/// Shared by `complete_step_and_advance` (a function/skip step finished) and
+/// `resume_finished_children` (a parked sub-workflow step finished).
+pub async fn advance_run_tx(
+    tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
+    run_id: WorkflowRunId,
+    definition: &WorkflowDefinition,
+) -> Result<(), WorkflowRepoError> {
     let run: Option<(String,)> =
         sqlx::query_as("SELECT status FROM workflow_runs WHERE id = $1 FOR UPDATE")
-            .bind(claimed.run_id.into_inner())
-            .fetch_optional(&mut *tx)
+            .bind(run_id.into_inner())
+            .fetch_optional(&mut **tx)
             .await?;
     let Some((run_status,)) = run else {
-        tx.commit().await?;
-        return Ok(AdvanceResult::Advanced);
+        return Ok(());
     };
     if RunStatus::from_str(&run_status).is_some_and(RunStatus::is_terminal) {
         // Another worker already finished the run; nothing to do.
-        tx.commit().await?;
-        return Ok(AdvanceResult::Advanced);
+        return Ok(());
     }
 
     let step_rows: Vec<StepStateRow> = sqlx::query_as(
         "SELECT step_name, status, output FROM workflow_run_steps WHERE run_id = $1",
     )
-    .bind(claimed.run_id.into_inner())
-    .fetch_all(&mut *tx)
+    .bind(run_id.into_inner())
+    .fetch_all(&mut **tx)
     .await?;
     let mut statuses = BTreeMap::new();
     let mut outputs = BTreeMap::new();
@@ -717,7 +744,7 @@ pub async fn complete_step_and_advance(
         }
     }
 
-    let plan = compute_advance(&claimed.definition, &statuses, &outputs);
+    let plan = compute_advance(definition, &statuses, &outputs);
 
     for name in &plan.promote {
         sqlx::query(
@@ -725,9 +752,9 @@ pub async fn complete_step_and_advance(
              SET status = 'ready', next_attempt_at = NULL, updated_at = NOW() \
              WHERE run_id = $1 AND step_name = $2 AND status = 'pending'",
         )
-        .bind(claimed.run_id.into_inner())
+        .bind(run_id.into_inner())
         .bind(name)
-        .execute(&mut *tx)
+        .execute(&mut **tx)
         .await?;
     }
 
@@ -737,9 +764,9 @@ pub async fn complete_step_and_advance(
                 "UPDATE workflow_runs \
                  SET status = 'succeeded', output = $2, finished_at = NOW() WHERE id = $1",
             )
-            .bind(claimed.run_id.into_inner())
+            .bind(run_id.into_inner())
             .bind(plan.run_output)
-            .execute(&mut *tx)
+            .execute(&mut **tx)
             .await?;
         }
         RunStatus::Failed => {
@@ -747,9 +774,9 @@ pub async fn complete_step_and_advance(
                 "UPDATE workflow_runs \
                  SET status = 'failed', error = $2, finished_at = NOW() WHERE id = $1",
             )
-            .bind(claimed.run_id.into_inner())
+            .bind(run_id.into_inner())
             .bind(plan.run_error)
-            .execute(&mut *tx)
+            .execute(&mut **tx)
             .await?;
         }
         _ => {
@@ -757,14 +784,13 @@ pub async fn complete_step_and_advance(
                 "UPDATE workflow_runs \
                  SET status = 'running', started_at = COALESCE(started_at, NOW()) WHERE id = $1",
             )
-            .bind(claimed.run_id.into_inner())
-            .execute(&mut *tx)
+            .bind(run_id.into_inner())
+            .execute(&mut **tx)
             .await?;
         }
     }
 
-    tx.commit().await?;
-    Ok(AdvanceResult::Advanced)
+    Ok(())
 }
 
 /// Periodic safety net: a step leased by a crashed worker (still `running`
@@ -924,6 +950,161 @@ pub async fn load_run_step_outputs(
         .collect())
 }
 
+// ---- nested sub-workflows (M4) --------------------------------------------
+
+/// Resolve an app-owned workflow by case-insensitive name (a `workflow`-kind
+/// step's target). Pool free-fn mirror of `WorkflowRepo::get_by_name`.
+pub async fn get_workflow_by_name(
+    pool: &PgPool,
+    app_id: AppId,
+    name: &str,
+) -> Result<Option<Workflow>, WorkflowRepoError> {
+    let row: Option<WorkflowRow> = sqlx::query_as(&format!(
+        "SELECT {SELECT_COLS} FROM workflows WHERE app_id = $1 AND LOWER(name) = LOWER($2)"
+    ))
+    .bind(app_id.into_inner())
+    .bind(name)
+    .fetch_optional(pool)
+    .await?;
+    row.map(TryInto::try_into).transpose()
+}
+
+/// Start a nested sub-workflow run and **park** the parent step on it. In one
+/// token-gated transaction: the parent step (still claimed by the caller) is
+/// set `running` with `claim_token` cleared and `child_run_id` pointing at a
+/// freshly-seeded child run (depth + 1, parent linkage). The parked step is
+/// never re-claimed (only `ready` steps are) nor reclaimed (only leased
+/// `running` steps — `claim_token` is NULL here); `resume_finished_children`
+/// resolves it once the child terminates. A stale worker matches zero rows and
+/// nothing (including the child) is written.
+pub async fn start_child_and_park(
+    pool: &PgPool,
+    claimed: &ClaimedStep,
+    child_workflow_id: WorkflowId,
+    child_definition: &WorkflowDefinition,
+    child_input: serde_json::Value,
+) -> Result<AdvanceResult, WorkflowRepoError> {
+    let mut tx = pool.begin().await?;
+
+    // Token-gated park FIRST — so a stale claim short-circuits before we ever
+    // insert an orphan child run (child_run_id is linked below).
+    let res = sqlx::query(
+        "UPDATE workflow_run_steps \
+         SET status = 'running', claim_token = NULL, claimed_at = NULL, updated_at = NOW() \
+         WHERE id = $1 AND claim_token = $2",
+    )
+    .bind(claimed.step_id.into_inner())
+    .bind(claimed.claim_token)
+    .execute(&mut *tx)
+    .await?;
+    if res.rows_affected() == 0 {
+        tx.rollback().await?;
+        return Ok(AdvanceResult::Stale);
+    }
+
+    let new = NewWorkflowRun {
+        workflow_id: child_workflow_id,
+        app_id: claimed.app_id,
+        input: child_input,
+        // Correlate the whole nested tree under the top-level run's root id.
+        root_execution_id: claimed.root_execution_id,
+        workflow_depth: claimed.workflow_depth + 1,
+        parent_run_id: Some(claimed.run_id),
+        parent_step_id: Some(claimed.step_id),
+    };
+    let child_run = seed_run_tx(&mut tx, &new, child_definition).await?;
+
+    sqlx::query(
+        "UPDATE workflow_run_steps SET child_run_id = $2, updated_at = NOW() WHERE id = $1",
+    )
+    .bind(claimed.step_id.into_inner())
+    .bind(child_run.into_inner())
+    .execute(&mut *tx)
+    .await?;
+
+    sqlx::query(
+        "UPDATE workflow_runs SET status = 'running', started_at = COALESCE(started_at, NOW()) \
+         WHERE id = $1 AND status = 'pending'",
+    )
+    .bind(claimed.run_id.into_inner())
+    .execute(&mut *tx)
+    .await?;
+
+    tx.commit().await?;
+    Ok(AdvanceResult::Advanced)
+}
+
+#[derive(sqlx::FromRow)]
+struct ParkedRow {
+    parent_step_id: Uuid,
+    parent_run_id: Uuid,
+    child_status: String,
+    child_output: Option<serde_json::Value>,
+    child_error: Option<String>,
+    parent_definition: serde_json::Value,
+}
+
+/// Resolve parent steps parked on a now-terminal child sub-workflow: write the
+/// child's output (or error) onto the parent step and advance the parent run.
+/// Idempotent — the conditional `status = 'running'` gate means a second pass
+/// (or a second worker) matches zero rows. Returns the number resolved.
+pub async fn resume_finished_children(pool: &PgPool) -> Result<u64, WorkflowRepoError> {
+    let rows: Vec<ParkedRow> = sqlx::query_as(
+        "SELECT ps.id AS parent_step_id, ps.run_id AS parent_run_id, \
+                child.status AS child_status, child.output AS child_output, \
+                child.error AS child_error, w.definition AS parent_definition \
+         FROM workflow_run_steps ps \
+         JOIN workflow_runs child ON child.id = ps.child_run_id \
+         JOIN workflow_runs parent ON parent.id = ps.run_id \
+         JOIN workflows w ON w.id = parent.workflow_id \
+         WHERE ps.status = 'running' AND ps.child_run_id IS NOT NULL \
+           AND ps.claim_token IS NULL \
+           AND child.status IN ('succeeded', 'failed', 'canceled') \
+         LIMIT 32",
+    )
+    .fetch_all(pool)
+    .await?;
+
+    let mut resolved = 0u64;
+    for r in rows {
+        let mut tx = pool.begin().await?;
+        let (new_status, out, err) = if r.child_status == "succeeded" {
+            ("succeeded", r.child_output.clone(), None)
+        } else {
+            (
+                "failed",
+                None,
+                Some(
+                    r.child_error
+                        .clone()
+                        .unwrap_or_else(|| "sub-workflow failed".to_string()),
+                ),
+            )
+        };
+        let res = sqlx::query(
+            "UPDATE workflow_run_steps \
+             SET status = $2, output = $3, error = $4, updated_at = NOW() \
+             WHERE id = $1 AND status = 'running' AND child_run_id IS NOT NULL",
+        )
+        .bind(r.parent_step_id)
+        .bind(new_status)
+        .bind(&out)
+        .bind(&err)
+        .execute(&mut *tx)
+        .await?;
+        if res.rows_affected() == 0 {
+            tx.rollback().await?;
+            continue; // already resolved by another pass
+        }
+        let parent_def: WorkflowDefinition = serde_json::from_value(r.parent_definition)
+            .map_err(|e| WorkflowRepoError::Serde(e.to_string()))?;
+        advance_run_tx(&mut tx, r.parent_run_id.into(), &parent_def).await?;
+        tx.commit().await?;
+        resolved += 1;
+    }
+    Ok(resolved)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/crates/manager-core/tests/workflow_orchestrator.rs b/crates/manager-core/tests/workflow_orchestrator.rs
index b16e041..8d90c29 100644
--- a/crates/manager-core/tests/workflow_orchestrator.rs
+++ b/crates/manager-core/tests/workflow_orchestrator.rs
@@ -616,12 +616,13 @@ async fn seed_scripts(pool: &PgPool, app: AppId, names: &[&str]) {
     }
 }
 
-/// Drive a real orchestrator to terminal using `executor`.
-async fn drive_to_terminal(
+/// Drive a real orchestrator to terminal using `executor` + `config`.
+async fn drive_with(
     pool: &PgPool,
     executor: Arc<dyn ExecutorClient>,
     app: AppId,
     run: picloud_shared::WorkflowRunId,
+    config: WorkflowOrchestratorConfig,
 ) {
     let orch = WorkflowOrchestrator {
         pool: pool.clone(),
@@ -629,9 +630,9 @@ async fn drive_to_terminal(
         executor,
         gate: Arc::new(ExecutionGate::new(8)),
         log_sink: Arc::new(NoopLogSink),
-        config: WorkflowOrchestratorConfig::default(),
+        config,
     };
-    for _ in 0..50 {
+    for _ in 0..80 {
         orch.tick_once().await;
         if get_run(pool, app, run)
             .await
@@ -645,6 +646,76 @@ async fn drive_to_terminal(
     }
 }
 
+/// Drive to terminal with the default config.
+async fn drive_to_terminal(
+    pool: &PgPool,
+    executor: Arc<dyn ExecutorClient>,
+    app: AppId,
+    run: picloud_shared::WorkflowRunId,
+) {
+    drive_with(
+        pool,
+        executor,
+        app,
+        run,
+        WorkflowOrchestratorConfig::default(),
+    )
+    .await;
+}
+
+/// A `workflow`-kind (nested sub-workflow) step.
+fn wf_step(name: &str, workflow: &str, deps: &[&str]) -> WorkflowStepDef {
+    WorkflowStepDef {
+        name: name.into(),
+        function: None,
+        workflow: Some(workflow.into()),
+        input: Value::Null,
+        depends_on: deps.iter().map(|s| (*s).to_string()).collect(),
+        when: None,
+        retry: None,
+        on_error: OnError::Fail,
+    }
+}
+
+/// Create just a group + app; return the app id.
+async fn seed_app(pool: &PgPool) -> AppId {
+    let sfx = Uuid::new_v4().simple().to_string();
+    let grp: (Uuid,) =
+        sqlx::query_as("INSERT INTO groups (slug, name) VALUES ($1, $1) RETURNING id")
+            .bind(format!("wf-grp-{sfx}"))
+            .fetch_one(pool)
+            .await
+            .expect("insert group");
+    let app: (Uuid,) =
+        sqlx::query_as("INSERT INTO apps (slug, name, group_id) VALUES ($1, $1, $2) RETURNING id")
+            .bind(format!("wf-app-{sfx}"))
+            .bind(grp.0)
+            .fetch_one(pool)
+            .await
+            .expect("insert app");
+    app.0.into()
+}
+
+/// Insert a workflow with a specific name into an existing app.
+async fn seed_named_workflow(
+    pool: &PgPool,
+    app: AppId,
+    name: &str,
+    def: &WorkflowDefinition,
+) -> WorkflowId {
+    let def_json = serde_json::to_value(def).unwrap();
+    let wf: (Uuid,) = sqlx::query_as(
+        "INSERT INTO workflows (app_id, name, definition) VALUES ($1, $2, $3) RETURNING id",
+    )
+    .bind(app.into_inner())
+    .bind(name)
+    .bind(def_json)
+    .fetch_one(pool)
+    .await
+    .expect("insert named workflow");
+    wf.0.into()
+}
+
 fn run_with_input(app: AppId, wf: WorkflowId, input: Value) -> NewWorkflowRun {
     NewWorkflowRun {
         workflow_id: wf,
@@ -762,3 +833,97 @@ async fn missing_input_ref_fails_the_step() {
     assert_eq!(bstep.status, StepStatus::Failed);
     assert!(bstep.error.as_deref().unwrap_or("").contains("input"));
 }
+
+// ===========================================================================
+// M4 — nested sub-workflows.
+// ===========================================================================
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn nested_sub_workflow_runs_and_output_flows_to_parent() {
+    let Some(pool) = pool_or_skip().await else {
+        return;
+    };
+    let _claim_guard = lock_and_reset(&pool).await;
+    let app = seed_app(&pool).await;
+
+    // Child workflow: one function step producing { answer: 42 }.
+    let child_def = WorkflowDefinition {
+        steps: vec![step("leaf", "fn_leaf", &[])],
+    };
+    seed_named_workflow(&pool, app, "child", &child_def).await;
+
+    // Parent: a `workflow`-kind step "call" → child, then "after" consuming its
+    // output. The child run's output is { leaf: <leaf output> }, so
+    // steps.call.output.leaf.answer reaches through the nesting.
+    let mut after = step("after", "fn_after", &["call"]);
+    after.input = json!({ "got": "{{ steps.call.output.leaf.answer }}" });
+    let parent_def = WorkflowDefinition {
+        steps: vec![wf_step("call", "child", &[]), after],
+    };
+    let parent_wf = seed_named_workflow(&pool, app, "parent", &parent_def).await;
+    seed_scripts(&pool, app, &["fn_leaf", "fn_after"]).await;
+
+    let run = start_run(&pool, new_run(app, parent_wf), &parent_def)
+        .await
+        .expect("start");
+
+    let executor = Arc::new(ScriptedExecutor::new(&[(
+        "fn_leaf",
+        json!({ "answer": 42 }),
+    )]));
+    drive_with(
+        &pool,
+        executor.clone(),
+        app,
+        run,
+        WorkflowOrchestratorConfig::default(),
+    )
+    .await;
+
+    let r = get_run(&pool, app, run).await.unwrap().unwrap();
+    assert_eq!(r.status, RunStatus::Succeeded);
+    // The child's output flowed into the parent step, then into `after`'s input.
+    assert_eq!(executor.body_for("fn_after"), Some(json!({ "got": 42 })));
+    // The parent step "call" carries the child run's output.
+    let steps = list_run_steps(&pool, run).await.unwrap();
+    let call = steps.iter().find(|s| s.step_name == "call").unwrap();
+    assert_eq!(call.status, StepStatus::Succeeded);
+    assert_eq!(call.output, Some(json!({ "leaf": { "answer": 42 } })));
+    assert!(
+        call.child_run_id.is_some(),
+        "parent step links its child run"
+    );
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn nesting_depth_ceiling_fails_the_run() {
+    let Some(pool) = pool_or_skip().await else {
+        return;
+    };
+    let _claim_guard = lock_and_reset(&pool).await;
+    let app = seed_app(&pool).await;
+
+    // A workflow that nests into ITSELF — bounded only by the depth ceiling.
+    let def = WorkflowDefinition {
+        steps: vec![wf_step("recurse", "loopwf", &[])],
+    };
+    let wf = seed_named_workflow(&pool, app, "loopwf", &def).await;
+    let run = start_run(&pool, new_run(app, wf), &def)
+        .await
+        .expect("start");
+
+    // max_depth = 2 → depth 0 and 1 spawn children; depth 2's spawn is refused.
+    let config = WorkflowOrchestratorConfig {
+        max_depth: 2,
+        ..WorkflowOrchestratorConfig::default()
+    };
+    let executor = Arc::new(ScriptedExecutor::new(&[]));
+    drive_with(&pool, executor, app, run, config).await;
+
+    let r = get_run(&pool, app, run).await.unwrap().unwrap();
+    assert_eq!(
+        r.status,
+        RunStatus::Failed,
+        "self-nesting run fails at the depth ceiling, not infinitely"
+    );
+}