From d1812751dd759f3127132fc90e3ced57ac38c282 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 18 Apr 2026 01:56:01 -0500 Subject: [PATCH] fix(interpreter): keep repl alive after child exit --- crates/bashkit/src/interpreter/mod.rs | 45 +++--- crates/bashkit/src/lib.rs | 211 ++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 19 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index e02d17dd..4549ba1b 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -28,6 +28,11 @@ use std::sync::{Arc, Mutex as StdMutex}; /// Monotonic counter for unique process substitution file paths static PROC_SUB_COUNTER: AtomicU64 = AtomicU64::new(0); +// Important decision: report a bash-compatible version surface instead of the +// bashkit crate semver so scripts that gate on Bash features keep working. +const COMPAT_BASH_VERSION: &str = "5.2.15(1)-release"; +const COMPAT_BASH_VERSINFO: [&str; 6] = ["5", "2", "15", "1", "release", "virtual"]; + use futures_util::FutureExt; use crate::builtins::{self, Builtin}; @@ -52,6 +57,14 @@ pub struct HistoryEntry { pub duration_ms: u64, } +fn compat_bash_versinfo_array() -> HashMap { + COMPAT_BASH_VERSINFO + .iter() + .enumerate() + .map(|(idx, value)| (idx, (*value).to_string())) + .collect() +} + /// Callback for streaming output chunks as they are produced. /// /// Arguments: `(stdout_chunk, stderr_chunk)`. Called after each loop iteration @@ -960,18 +973,8 @@ impl Interpreter { variables.insert("HOSTNAME".to_string(), hostname_val.clone()); // BASH_VERSINFO array: (major minor patch build status machine) - let version = env!("CARGO_PKG_VERSION"); - let parts: Vec<&str> = version.split('.').collect(); - let mut bash_versinfo = HashMap::new(); - bash_versinfo.insert(0, parts.first().unwrap_or(&"0").to_string()); - bash_versinfo.insert(1, parts.get(1).unwrap_or(&"0").to_string()); - bash_versinfo.insert(2, parts.get(2).unwrap_or(&"0").to_string()); - bash_versinfo.insert(3, "0".to_string()); - bash_versinfo.insert(4, "release".to_string()); - bash_versinfo.insert(5, "virtual".to_string()); - let mut arrays = HashMap::new(); - arrays.insert("BASH_VERSINFO".to_string(), bash_versinfo); + arrays.insert("BASH_VERSINFO".to_string(), compat_bash_versinfo_array()); // Seed PRNG for $RANDOM from OS entropy via RandomState let random_seed = { @@ -1441,7 +1444,7 @@ impl Interpreter { .check_session_limits(&self.session_limits) .map_err(|e| crate::error::Error::Execution(e.to_string()))?; - self.execute_script_body(script, true).await + self.execute_script_body(script, true, true).await } /// Clean up process substitution temp files (`/dev/fd/proc_sub_*`). @@ -1459,12 +1462,14 @@ impl Interpreter { } /// Inner script execution — runs commands without resetting counters. - /// Used by `execute_source` to preserve function/source depth tracking. - /// `run_exit_trap`: only the top-level `execute` runs the EXIT trap. + /// Used by `execute_source` and nested shell contexts. + /// `run_exit_trap`: whether this shell context runs its EXIT trap. + /// `fire_exit_hook`: whether `exit` notifies host-level on_exit hooks. async fn execute_script_body( &mut self, script: &Script, run_exit_trap: bool, + fire_exit_hook: bool, ) -> Result { let mut stdout = String::new(); let mut stderr = String::new(); @@ -1515,7 +1520,7 @@ impl Interpreter { // Stop on control flow (e.g. nounset error uses Return to abort) if result.control_flow != ControlFlow::None { - if let ControlFlow::Exit(code) = result.control_flow { + if fire_exit_hook && let ControlFlow::Exit(code) = result.control_flow { self.hooks.fire_on_exit(crate::hooks::ExitEvent { code }); } break; @@ -3140,7 +3145,7 @@ impl Interpreter { self.update_bash_source(); } - let result = self.execute(&script).await; + let result = self.execute_script_body(&script, true, false).await; // Restore BASH_SOURCE if script_file.is_some() { @@ -4762,6 +4767,8 @@ impl Interpreter { // Clear nounset_error to prevent parent expansion errors from leaking. self.variables = self.env.clone(); self.arrays.clear(); + self.arrays + .insert("BASH_VERSINFO".to_string(), compat_bash_versinfo_array()); self.assoc_arrays.clear(); self.functions.clear(); self.traps.clear(); @@ -4786,7 +4793,7 @@ impl Interpreter { let prev_pipeline_stdin = self.pipeline_stdin.take(); self.pipeline_stdin = stdin; - let result = self.execute(&script).await; + let result = self.execute_script_body(&script, true, false).await; // Restore full parent state — child mutations don't propagate self.variables = saved_vars; @@ -4937,7 +4944,7 @@ impl Interpreter { // Execute the script commands in the current shell context. // Use execute_script_body (not execute) to preserve depth counters. - let exec_result = self.execute_script_body(&script, false).await; + let exec_result = self.execute_script_body(&script, false, true).await; // Pop source depth and BASH_SOURCE (always, even on error) self.counters.pop_function(); @@ -9300,7 +9307,7 @@ impl Interpreter { return "localhost".to_string(); } "BASH_VERSION" => { - return format!("{}-bashkit", env!("CARGO_PKG_VERSION")); + return COMPAT_BASH_VERSION.to_string(); } "SECONDS" => { // Seconds since shell started - always 0 in stateless model diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index 6ad36a75..6a90b78a 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -6232,6 +6232,217 @@ echo missing fi"#, assert_eq!(result.stdout.trim(), "greetings hooks"); } + #[tokio::test] + async fn test_on_exit_hook_not_fired_for_path_script_exit() { + use std::path::Path; + use std::sync::Arc; + use std::sync::atomic::{AtomicU32, Ordering}; + + let count = Arc::new(AtomicU32::new(0)); + let count_clone = count.clone(); + + let mut bash = Bash::builder() + .on_exit(Box::new(move |event| { + count_clone.fetch_add(1, Ordering::Relaxed); + hooks::HookAction::Continue(event) + })) + .build(); + + let fs = bash.fs(); + fs.mkdir(Path::new("/bin"), false).await.unwrap(); + fs.write_file(Path::new("/bin/child-exit"), b"#!/usr/bin/env bash\nexit 7") + .await + .unwrap(); + fs.chmod(Path::new("/bin/child-exit"), 0o755).await.unwrap(); + + let result = bash + .exec("PATH=/bin:$PATH\nchild-exit\necho after:$?") + .await + .unwrap(); + + assert_eq!(result.stdout.trim(), "after:7"); + assert_eq!(count.load(Ordering::Relaxed), 0); + } + + #[tokio::test] + async fn test_on_exit_hook_not_fired_for_direct_script_exit() { + use std::path::Path; + use std::sync::Arc; + use std::sync::atomic::{AtomicU32, Ordering}; + + let count = Arc::new(AtomicU32::new(0)); + let count_clone = count.clone(); + + let mut bash = Bash::builder() + .on_exit(Box::new(move |event| { + count_clone.fetch_add(1, Ordering::Relaxed); + hooks::HookAction::Continue(event) + })) + .build(); + + let fs = bash.fs(); + fs.write_file( + Path::new("/tmp/child-exit.sh"), + b"#!/usr/bin/env bash\nexit 8", + ) + .await + .unwrap(); + fs.chmod(Path::new("/tmp/child-exit.sh"), 0o755) + .await + .unwrap(); + + let result = bash + .exec("/tmp/child-exit.sh\necho after:$?") + .await + .unwrap(); + + assert_eq!(result.stdout.trim(), "after:8"); + assert_eq!(count.load(Ordering::Relaxed), 0); + } + + #[tokio::test] + async fn test_on_exit_hook_not_fired_for_nested_bash_exit() { + use std::sync::Arc; + use std::sync::atomic::{AtomicU32, Ordering}; + + let count = Arc::new(AtomicU32::new(0)); + let count_clone = count.clone(); + + let mut bash = Bash::builder() + .on_exit(Box::new(move |event| { + count_clone.fetch_add(1, Ordering::Relaxed); + hooks::HookAction::Continue(event) + })) + .build(); + + let result = bash.exec("bash -c 'exit 9'\necho after:$?").await.unwrap(); + + assert_eq!(result.stdout.trim(), "after:9"); + assert_eq!(count.load(Ordering::Relaxed), 0); + } + + #[tokio::test] + async fn test_path_script_exit_runs_child_exit_trap() { + use std::path::Path; + + let mut bash = Bash::new(); + let fs = bash.fs(); + fs.write_file( + Path::new("/tmp/child-trap.sh"), + b"#!/usr/bin/env bash\ntrap 'echo child-trap' EXIT\nexit 4", + ) + .await + .unwrap(); + fs.chmod(Path::new("/tmp/child-trap.sh"), 0o755) + .await + .unwrap(); + + let result = bash + .exec("/tmp/child-trap.sh\necho after:$?") + .await + .unwrap(); + + assert_eq!(result.stdout.trim(), "child-trap\nafter:4"); + } + + #[tokio::test] + async fn test_on_exit_hook_still_fires_for_source_exit() { + use std::path::Path; + use std::sync::Arc; + use std::sync::atomic::{AtomicU32, Ordering}; + + let count = Arc::new(AtomicU32::new(0)); + let count_clone = count.clone(); + + let mut bash = Bash::builder() + .on_exit(Box::new(move |event| { + count_clone.fetch_add(1, Ordering::Relaxed); + hooks::HookAction::Continue(event) + })) + .build(); + + let fs = bash.fs(); + fs.write_file(Path::new("/tmp/source-exit.sh"), b"exit 5") + .await + .unwrap(); + + let result = bash.exec("source /tmp/source-exit.sh").await.unwrap(); + + assert_eq!(result.exit_code, 5); + assert_eq!(count.load(Ordering::Relaxed), 1); + } + + #[tokio::test] + async fn test_bash_versinfo_reports_bash_compatible_major() { + let mut bash = Bash::new(); + + let result = bash + .exec(r#"[[ ${BASH_VERSINFO[0]} -ge 4 ]] && echo bash4plus"#) + .await + .unwrap(); + + assert_eq!(result.stdout.trim(), "bash4plus"); + } + + #[tokio::test] + async fn test_bash_version_surface_matches_bash_compatible_tuple() { + let mut bash = Bash::new(); + + let result = bash + .exec( + r#"printf '%s\n' "$BASH_VERSION" "${BASH_VERSINFO[0]}" "${BASH_VERSINFO[1]}" "${BASH_VERSINFO[2]}" "${BASH_VERSINFO[3]}" "${BASH_VERSINFO[4]}" "${BASH_VERSINFO[5]}""#, + ) + .await + .unwrap(); + + assert_eq!( + result.stdout, + "5.2.15(1)-release\n5\n2\n15\n1\nrelease\nvirtual\n" + ); + } + + #[tokio::test] + async fn test_path_script_retains_bash_versinfo_array() { + use std::path::Path; + + let mut bash = Bash::new(); + let fs = bash.fs(); + fs.write_file( + Path::new("/tmp/bash-version-check.sh"), + b"#!/usr/bin/env bash\nprintf '%s\\n' \"${BASH_VERSINFO[0]}\"", + ) + .await + .unwrap(); + fs.chmod(Path::new("/tmp/bash-version-check.sh"), 0o755) + .await + .unwrap(); + + let result = bash.exec("/tmp/bash-version-check.sh").await.unwrap(); + + assert_eq!(result.stdout.trim(), "5"); + } + + #[tokio::test] + async fn test_path_script_bash_versinfo_satisfies_bash4_guard() { + use std::path::Path; + + let mut bash = Bash::new(); + let fs = bash.fs(); + fs.write_file( + Path::new("/tmp/bash-version-guard.sh"), + b"#!/usr/bin/env bash\nif (( BASH_VERSINFO[0] < 4 )); then echo too-old; else echo ok; fi", + ) + .await + .unwrap(); + fs.chmod(Path::new("/tmp/bash-version-guard.sh"), 0o755) + .await + .unwrap(); + + let result = bash.exec("/tmp/bash-version-guard.sh").await.unwrap(); + + assert_eq!(result.stdout.trim(), "ok"); + } + #[tokio::test] async fn test_before_tool_hook_modifies_args() { use std::sync::Arc;