fix(interpreter): loop/return scoping, trap listings, [[ ]] operands and builtin parity from the BashBox corpus

crates/bashkit/src/builtins/alias.rs

@@ -4,6 +4,7 @@
 
 use async_trait::async_trait;
 
+use super::helpers::single_quote;
 use super::{Builtin, Context};
 use crate::error::Result;
 use crate::interpreter::ExecResult;
@@ -29,7 +30,7 @@ impl Builtin for Alias {
             sorted.sort_by_key(|(k, _)| (*k).clone());
             let mut output = String::new();
             for (name, value) in sorted {
-                output.push_str(&format!("alias {}='{}'\n", name, value));
+                output.push_str(&format!("alias {name}={}\n", single_quote(value)));
             }
             return Ok(ExecResult::ok(output));
         }
@@ -47,7 +48,7 @@ impl Builtin for Alias {
             } else {
                 // alias name — show the alias
                 if let Some(value) = shell.aliases.get(arg.as_str()) {
-                    output.push_str(&format!("alias {}='{}'\n", arg, value));
+                    output.push_str(&format!("alias {arg}={}\n", single_quote(value)));
                 } else {
                     stderr.push_str(&format!("bash: alias: {}: not found\n", arg));
                     exit_code = 1;

crates/bashkit/src/builtins/dirstack.rs

@@ -24,15 +24,37 @@ fn normalize_path(base: &std::path::Path, target: &str) -> PathBuf {
     super::resolve_path(&PathBuf::from("/"), &path.to_string_lossy())
 }
 
-/// Format the stack as bash's `dirs` default: current dir followed by the
-/// stack from top (most recent) to bottom.
-fn format_stack(ctx: &Context<'_>) -> String {
-    let cwd = ctx.cwd.to_string_lossy().to_string();
-    let mut parts = vec![cwd];
+/// bash `polite_directory_format`: a directory under `$HOME` is shown as
+/// `~` plus the rest (`$HOME` of `/` or unset disables it).
+fn polite(dir: &str, home: Option<&str>) -> String {
+    if let Some(home) = home.filter(|h| h.len() > 1)
+        && let Some(rest) = dir.strip_prefix(home)
+        && (rest.is_empty() || rest.starts_with('/'))
+    {
+        return format!("~{rest}");
+    }
+    dir.to_string()
+}
+
+/// Current dir followed by the stack from top (most recent) to bottom,
+/// `~`-abbreviated unless `long`.
+fn stack_entries(ctx: &Context<'_>, long: bool) -> Vec<String> {
+    let home = if long {
+        None
+    } else {
+        ctx.variables.get("HOME").map(String::as_str)
+    };
+    let cwd = ctx.cwd.to_string_lossy();
+    let mut parts = vec![polite(&cwd, home)];
     if let Some(shell) = ctx.shell.as_ref() {
-        parts.extend(shell.dir_stack.iter().rev().cloned());
+        parts.extend(shell.dir_stack.iter().rev().map(|d| polite(d, home)));
     }
-    parts.join(" ")
+    parts
+}
+
+/// Format the stack as bash's `dirs` default: one line, `~`-abbreviated.
+fn format_stack(ctx: &Context<'_>) -> String {
+    stack_entries(ctx, false).join(" ")
 }
 
 /// The pushd builtin - push directory onto stack and cd.
@@ -145,6 +167,7 @@ impl Builtin for Dirs {
         let mut clear = false;
         let mut per_line = false;
         let mut verbose = false;
+        let mut long = false;
 
         for arg in ctx.args.iter() {
             match arg.as_str() {
@@ -154,7 +177,7 @@ impl Builtin for Dirs {
                     verbose = true;
                     per_line = true;
                 }
-                "-l" => {} // long listing (we don't do ~ substitution anyway)
+                "-l" => long = true,
                 _ => {}
             }
         }
@@ -166,28 +189,18 @@ impl Builtin for Dirs {
             return Ok(ExecResult::ok(String::new()));
         }
 
-        if !verbose && !per_line {
-            // Default output doesn't need to walk the stack separately.
-            return Ok(ExecResult::ok(format!("{}\n", format_stack(&ctx))));
-        }
-
-        let cwd = ctx.cwd.to_string_lossy().to_string();
-        // Stack from top (most recent) to bottom, borrowed (no clone).
-        let empty: Vec<String> = Vec::new();
-        let stack = ctx.shell.as_ref().map(|s| &*s.dir_stack).unwrap_or(&empty);
-
-        if verbose {
-            let mut output = format!(" 0  {}\n", cwd);
-            for (n, dir) in stack.iter().rev().enumerate() {
-                output.push_str(&format!(" {}  {}\n", n + 1, dir));
-            }
-            Ok(ExecResult::ok(output))
+        let entries = stack_entries(&ctx, long);
+        let output = if verbose {
+            entries
+                .iter()
+                .enumerate()
+                .map(|(n, dir)| format!(" {n}  {dir}\n"))
+                .collect()
+        } else if per_line {
+            entries.iter().map(|dir| format!("{dir}\n")).collect()
         } else {
-            let mut output = format!("{}\n", cwd);
-            for dir in stack.iter().rev() {
-                output.push_str(&format!("{}\n", dir));
-            }
-            Ok(ExecResult::ok(output))
-        }
+            format!("{}\n", entries.join(" "))
+        };
+        Ok(ExecResult::ok(output))
     }
 }

crates/bashkit/src/builtins/flow.rs

@@ -64,19 +64,55 @@ impl Builtin for Exit {
     }
 }
 
+/// Resolve the level count of `break`/`continue` against the enclosing loops.
+///
+/// Bash semantics: outside a loop the command only warns (status 0); a count
+/// below 1 is an error (status 1); a count above the loop depth leaves every
+/// enclosing loop of the current function, never a caller's loop (a function
+/// call starts outside any loop, so the count is clamped to its depth).
+fn loop_control(name: &str, ctx: &Context<'_>, make: fn(u32) -> ControlFlow) -> ExecResult {
+    // Builtins run without shell state (e.g. direct unit calls) keep the
+    // plain unwinding behaviour.
+    let depth = ctx.shell.as_ref().map_or(u32::MAX, |s| {
+        u32::try_from(s.loop_depth).unwrap_or(u32::MAX)
+    });
+    if depth == 0 {
+        return ExecResult::err(
+            format!("bash: {name}: only meaningful in a `for', `while', or `until' loop\n"),
+            0,
+        );
+    }
+    let levels = match ctx.args.first() {
+        None => 1,
+        Some(arg) => match arg.trim().parse::<i64>() {
+            Ok(n) if n >= 1 => n,
+            Ok(_) => {
+                // Bash still leaves every enclosing loop.
+                let mut result = ExecResult::with_control_flow(ControlFlow::Break(depth));
+                result.exit_code = 1;
+                result.stderr = format!("bash: {name}: {arg}: loop count out of range\n").into();
+                return result;
+            }
+            Err(_) => {
+                // A special-builtin usage error ends a non-interactive shell.
+                let mut result = ExecResult::with_control_flow(ControlFlow::Exit(128));
+                result.exit_code = 128;
+                result.stderr = format!("bash: {name}: {arg}: numeric argument required\n").into();
+                return result;
+            }
+        },
+    };
+    let levels = u32::try_from(levels).unwrap_or(u32::MAX).min(depth);
+    ExecResult::with_control_flow(make(levels))
+}
+
 /// The break builtin - break out of a loop
 pub struct Break;
 
 #[async_trait]
 impl Builtin for Break {
     async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
-        let levels = ctx
-            .args
-            .first()
-            .and_then(|s| s.parse::<u32>().ok())
-            .unwrap_or(1);
-
-        Ok(ExecResult::with_control_flow(ControlFlow::Break(levels)))
+        Ok(loop_control("break", &ctx, ControlFlow::Break))
     }
 }
 
@@ -86,33 +122,47 @@ pub struct Continue;
 #[async_trait]
 impl Builtin for Continue {
     async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
-        let levels = ctx
-            .args
-            .first()
-            .and_then(|s| s.parse::<u32>().ok())
-            .unwrap_or(1);
-
-        Ok(ExecResult::with_control_flow(ControlFlow::Continue(levels)))
+        Ok(loop_control("continue", &ctx, ControlFlow::Continue))
     }
 }
 
-/// The return builtin - return from a function.
+/// The return builtin - return from a function or sourced file.
 /// Bash truncates return codes to 8-bit unsigned range (0-255) via `& 0xFF`.
+/// Without an argument it returns `$?`; outside a function or sourced file
+/// it fails with status 2 and the script goes on.
 pub struct Return;
 
 #[async_trait]
 impl Builtin for Return {
     async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
-        let exit_code = ctx
-            .args
-            .first()
-            .and_then(|s| s.parse::<i32>().ok())
-            .unwrap_or(0)
-            & 0xFF;
-
-        Ok(ExecResult::with_control_flow(ControlFlow::Return(
-            exit_code,
-        )))
+        if let Some(shell) = ctx.shell.as_ref()
+            && shell.return_depth == 0
+        {
+            return Ok(ExecResult::err(
+                "bash: return: can only `return' from a function or sourced script\n",
+                2,
+            ));
+        }
+        let exit_code = match ctx.args.first() {
+            None => ctx.shell.as_ref().map_or(0, |s| s.last_exit_code),
+            Some(arg) => match arg.trim().parse::<i64>() {
+                Ok(n) => (n & 0xFF) as i32,
+                Err(_) => {
+                    let mut result = ExecResult::with_control_flow(ControlFlow::Return(2));
+                    result.exit_code = 2;
+                    result.stderr =
+                        format!("bash: return: {arg}: numeric argument required\n").into();
+                    return Ok(result);
+                }
+            },
+        };
+        if ctx.args.len() > 1 {
+            return Ok(ExecResult::err("bash: return: too many arguments\n", 1));
+        }
+
+        let mut result = ExecResult::with_control_flow(ControlFlow::Return(exit_code));
+        result.exit_code = exit_code;
+        Ok(result)
     }
 }
 
@@ -269,13 +319,14 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn break_non_numeric_defaults_one() {
+    async fn break_non_numeric_exits_the_shell() {
         let (fs, mut cwd, mut variables) = setup().await;
         let env = HashMap::new();
         let args = vec!["abc".to_string()];
         let ctx = Context::new_for_test(&args, &env, &mut variables, &mut cwd, fs.clone(), None);
         let result = Break.execute(ctx).await.unwrap();
-        assert!(matches!(result.control_flow, ControlFlow::Break(1)));
+        assert!(matches!(result.control_flow, ControlFlow::Exit(128)));
+        assert!(result.stderr.contains("numeric argument required"));
     }
 
     // ==================== continue ====================

crates/bashkit/src/builtins/helpers.rs

@@ -11,6 +11,12 @@
 use super::check_help_version;
 use crate::interpreter::ExecResult;
 
+/// bash `sh_single_quote`: wrap in single quotes, each `'` as `'\''`.
+/// Used by listings meant to be re-read as input (`alias`, `trap`).
+pub(crate) fn single_quote(s: &str) -> String {
+    format!("'{}'", s.replace('\'', "'\\''"))
+}
+
 /// Build the error real GNU coreutils/util-linux print for an unrecognized
 /// command-line option, so builtins reject typos the way the real tools do
 /// instead of silently ignoring the bad flag.

crates/bashkit/src/builtins/introspect.rs

@@ -189,16 +189,53 @@ impl Builtin for Which {
     }
 }
 
-/// `hash` builtin — no-op in sandboxed environment.
+/// `hash` builtin.
 ///
-/// In real bash, `hash` manages the command hash table for PATH lookups.
-/// In bashkit's sandboxed environment there is no real PATH search cache,
-/// so this is a no-op that always succeeds.
+/// Bashkit keeps no command hash table (every lookup walks the virtual
+/// PATH), so the table always reads as empty: a bare `hash` reports that,
+/// `hash -r` succeeds, and `hash NAME` only checks that NAME resolves.
 pub struct Hash;
 
 #[async_trait]
 impl Builtin for Hash {
-    async fn execute(&self, _ctx: Context<'_>) -> Result<ExecResult> {
-        Ok(ExecResult::ok(String::new()))
+    async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
+        let mut names = Vec::new();
+        let mut list_only = true;
+        let mut options_done = false;
+        for arg in ctx.args.iter() {
+            if !options_done && arg == "--" {
+                options_done = true;
+            } else if !options_done && arg.starts_with('-') && arg.len() > 1 {
+                // -r clears the (empty) table; -l/-t/-d/-p act on entries.
+                if arg.contains(['r', 'd', 'p', 't']) {
+                    list_only = false;
+                }
+            } else {
+                names.push(arg.as_str());
+            }
+        }
+        if names.is_empty() {
+            return Ok(if list_only {
+                ExecResult::ok("hash: hash table empty\n".to_string())
+            } else {
+                ExecResult::ok(String::new())
+            });
+        }
+        let mut errors = String::new();
+        for name in names {
+            let builtin = ctx
+                .shell
+                .as_ref()
+                .is_some_and(|s| s.has_builtin(name) || s.has_function(name));
+            if !builtin && search_path(&ctx, name).await.is_none() {
+                errors.push_str(&format!("bash: hash: {name}: not found\n"));
+            }
+        }
+        let code = i32::from(!errors.is_empty());
+        Ok(ExecResult {
+            stderr: errors.into(),
+            exit_code: code,
+            ..Default::default()
+        })
     }
 }

crates/bashkit/src/builtins/read.rs

@@ -56,7 +56,8 @@ impl Builtin for Read {
                             } else {
                                 &rest
                             };
-                            delimiter = delim_str.chars().next();
+                            // `-d ''` reads up to a NUL byte.
+                            delimiter = Some(delim_str.chars().next().unwrap_or('\0'));
                             break;
                         }
                         'n' | 'N' => {