From f100f1bc524da8bdb4fe82106e47bf8fc1996af9 Mon Sep 17 00:00:00 2001
From: Fernando Dingler <fdingler@amazon.com>
Date: Wed, 21 Oct 2026 05:18:55 +0000
Subject: [PATCH] fix: return real statuses from compounds in command
 substitution

A brace group, subshell, if, loop, or case run inside $(...) always
reported success, so a following && or || took the wrong branch and $?
read 0. Carry the status of the last command each compound ran, and set
$? after each item in the captured command line.
---
 src/exec.rs                |  53 +++++++---
 tests/shell_integration.rs | 202 +++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+), 15 deletions(-)

diff --git a/src/exec.rs b/src/exec.rs
index 1c9a853..3a26f12 100644
--- a/src/exec.rs
+++ b/src/exec.rs
@@ -1554,6 +1554,15 @@ async fn run_capturing(
     proc: &mut Process,
     command_line: &parser::CommandLine,
 ) -> String {
+    run_capturing_status(os, proc, command_line).await.0
+}
+
+/// [`run_capturing`], also returning the exit status of the last item it ran.
+async fn run_capturing_status(
+    os: Arc<dyn Kernel>,
+    proc: &mut Process,
+    command_line: &parser::CommandLine,
+) -> (String, i32) {
     let mut output = String::new();
     let mut last_exit = 0;
     let mut skip = false;
@@ -1576,8 +1585,8 @@ async fn run_capturing(
                     (exit, out)
                 }
                 Item::Group(cl) => {
-                    let s = Box::pin(run_capturing(os.clone(), proc, cl)).await;
-                    (0, s)
+                    let (s, exit) = Box::pin(run_capturing_status(os.clone(), proc, cl)).await;
+                    (exit, s)
                 }
                 Item::Subshell(cl) => {
                     let mut sub = proc.fork();
@@ -1586,8 +1595,9 @@ async fn run_capturing(
                         sub.err_msg(msg);
                         (1, String::new())
                     } else {
-                        let s = Box::pin(run_capturing(os.clone(), &mut sub, cl)).await;
-                        (0, s)
+                        let (s, exit) =
+                            Box::pin(run_capturing_status(os.clone(), &mut sub, cl)).await;
+                        (exit, s)
                     }
                 }
                 Item::If {
@@ -1599,43 +1609,54 @@ async fn run_capturing(
                     for (cond, body) in branches {
                         let exit = Box::pin(execute_command_line(os.clone(), proc, cond)).await;
                         if exit == 0 {
-                            let s = Box::pin(run_capturing(os.clone(), proc, body)).await;
-                            result = (0, s);
+                            let (s, exit) =
+                                Box::pin(run_capturing_status(os.clone(), proc, body)).await;
+                            result = (exit, s);
                             matched = true;
                             break;
                         }
                     }
                     if !matched && let Some(body) = else_body {
-                        let s = Box::pin(run_capturing(os.clone(), proc, body)).await;
-                        result = (0, s);
+                        let (s, exit) =
+                            Box::pin(run_capturing_status(os.clone(), proc, body)).await;
+                        result = (exit, s);
                     }
                     result
                 }
                 Item::While { condition, body } | Item::Until { condition, body } => {
                     let is_until = matches!(item, Item::Until { .. });
                     let mut s = String::new();
+                    let mut exit = 0;
                     loop {
                         let cond_exit =
                             Box::pin(execute_command_line(os.clone(), proc, condition)).await;
                         if (is_until && cond_exit == 0) || (!is_until && cond_exit != 0) {
                             break;
                         }
-                        s.push_str(&Box::pin(run_capturing(os.clone(), proc, body)).await);
+                        let (out, body_exit) =
+                            Box::pin(run_capturing_status(os.clone(), proc, body)).await;
+                        s.push_str(&out);
+                        exit = body_exit;
                     }
-                    (0, s)
+                    (exit, s)
                 }
                 Item::For { var, words, body } => {
                     let expanded = expand_words(os.clone(), proc, words).await;
                     let mut s = String::new();
+                    let mut exit = 0;
                     for val in &expanded {
                         proc.set_env(var, val);
-                        s.push_str(&Box::pin(run_capturing(os.clone(), proc, body)).await);
+                        let (out, body_exit) =
+                            Box::pin(run_capturing_status(os.clone(), proc, body)).await;
+                        s.push_str(&out);
+                        exit = body_exit;
                     }
-                    (0, s)
+                    (exit, s)
                 }
                 Item::Case { word, arms } => {
                     let value = expand_word(os.clone(), proc, word).await;
                     let mut s = String::new();
+                    let mut exit = 0;
                     for arm in arms {
                         let mut matched = false;
                         for pat in &arm.patterns {
@@ -1646,11 +1667,12 @@ async fn run_capturing(
                             }
                         }
                         if matched {
-                            s = Box::pin(run_capturing(os.clone(), proc, &arm.body)).await;
+                            (s, exit) =
+                                Box::pin(run_capturing_status(os.clone(), proc, &arm.body)).await;
                             break;
                         }
                     }
-                    (0, s)
+                    (exit, s)
                 }
                 Item::CompoundPipeline {
                     compound,
@@ -1736,6 +1758,7 @@ async fn run_capturing(
                 }
             };
             last_exit = exit;
+            proc.last_exit = exit;
             output.push_str(&out);
             if proc.max_output > 0 && output.len() > proc.max_output {
                 proc.err_msg("strands-shell: output size limit exceeded");
@@ -1745,7 +1768,7 @@ async fn run_capturing(
             }
         }
     }
-    output
+    (output, last_exit)
 }
 
 /// Public-facing execute_command_line that swallows break/continue.
diff --git a/tests/shell_integration.rs b/tests/shell_integration.rs
index 24739e4..5d8fb9d 100644
--- a/tests/shell_integration.rs
+++ b/tests/shell_integration.rs
@@ -3783,6 +3783,208 @@ expect!(
     "x y z"
 );
 
+// ── exec: compound status in command substitution ──────────────────
+expect!(
+    cmd_subst_group_false_and,
+    "X=$({ false; } && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_group_false_or,
+    "X=$({ false; } || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_group_false_status,
+    "X=$({ false; }; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_group_true_and,
+    "X=$({ true; } && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_group_true_or,
+    "X=$({ true; } || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_subshell_false_and,
+    "X=$( (false) && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_subshell_false_or,
+    "X=$( (false) || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_subshell_false_status,
+    "X=$( (false); echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_subshell_true_and,
+    "X=$( (true) && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_subshell_true_or,
+    "X=$( (true) || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_if_false_and,
+    "X=$(if true; then false; fi && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_if_false_or,
+    "X=$(if true; then false; fi || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_if_false_status,
+    "X=$(if true; then false; fi; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_if_true_and,
+    "X=$(if true; then true; fi && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_if_true_or,
+    "X=$(if true; then true; fi || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_for_false_and,
+    "X=$(for i in 1; do false; done && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_for_false_or,
+    "X=$(for i in 1; do false; done || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_for_false_status,
+    "X=$(for i in 1; do false; done; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_for_true_and,
+    "X=$(for i in 1; do true; done && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_for_true_or,
+    "X=$(for i in 1; do true; done || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_while_false_and,
+    "X=$(n=0; while [ $n -lt 1 ]; do n=1; false; done && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_while_false_or,
+    "X=$(n=0; while [ $n -lt 1 ]; do n=1; false; done || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_while_false_status,
+    "X=$(n=0; while [ $n -lt 1 ]; do n=1; false; done; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_while_true_and,
+    "X=$(n=0; while [ $n -lt 1 ]; do n=1; true; done && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_while_true_or,
+    "X=$(n=0; while [ $n -lt 1 ]; do n=1; true; done || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_until_false_and,
+    "X=$(n=0; until [ $n -ge 1 ]; do n=1; false; done && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_until_false_or,
+    "X=$(n=0; until [ $n -ge 1 ]; do n=1; false; done || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_until_false_status,
+    "X=$(n=0; until [ $n -ge 1 ]; do n=1; false; done; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_until_true_and,
+    "X=$(n=0; until [ $n -ge 1 ]; do n=1; true; done && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_until_true_or,
+    "X=$(n=0; until [ $n -ge 1 ]; do n=1; true; done || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_case_false_and,
+    "X=$(case a in (a) false;; esac && echo yes); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_case_false_or,
+    "X=$(case a in (a) false;; esac || echo no); echo \"[$X]\"",
+    "[no]"
+);
+expect!(
+    cmd_subst_case_false_status,
+    "X=$(case a in (a) false;; esac; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+expect!(
+    cmd_subst_case_true_and,
+    "X=$(case a in (a) true;; esac && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_case_true_or,
+    "X=$(case a in (a) true;; esac || echo no); echo \"[$X]\"",
+    "[]"
+);
+expect!(
+    cmd_subst_group_last_status_wins,
+    "X=$({ false; true; } && echo yes; { true; false; } || echo no); echo \"[$X]\"",
+    "[yes\nno]"
+);
+expect!(
+    cmd_subst_if_unmatched_is_success,
+    "X=$(if false; then false; fi && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_empty_loop_is_success,
+    "X=$(false; for i in; do false; done && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_case_unmatched_is_success,
+    "X=$(false; case a in (b) false;; esac && echo yes); echo \"[$X]\"",
+    "[yes]"
+);
+expect!(
+    cmd_subst_pipeline_status,
+    "X=$(false; echo $?); echo \"[$X]\"",
+    "[1]"
+);
+
 // ── exec: shebang / script execution ──────────────────────────────
 expect!(
     script_shebang_source,