diff --git a/crates/bashkit/src/builtins/ls.rs b/crates/bashkit/src/builtins/ls.rs index 722b7764..458e390b 100644 --- a/crates/bashkit/src/builtins/ls.rs +++ b/crates/bashkit/src/builtins/ls.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use std::path::Path; -use super::{Builtin, Context, resolve_path}; +use super::{Builtin, Context, ExecutionPlan, SubCommand, resolve_path}; use crate::error::Result; use crate::fs::FileType; use crate::interpreter::ExecResult; @@ -298,6 +298,10 @@ struct FindOptions { max_depth: Option, min_depth: Option, printf_format: Option, + /// -exec/-execdir command template (args before \; or +) + exec_args: Vec, + /// true if -exec uses + (batch mode), false for \; (per-file mode) + exec_batch: bool, } /// The find builtin - search for files. @@ -315,148 +319,286 @@ struct FindOptions { /// -exec CMD {} + Execute CMD once with all matches pub struct Find; -#[async_trait] -impl Builtin for Find { - async fn execute(&self, ctx: Context<'_>) -> Result { - let mut paths: Vec = Vec::new(); - let mut opts = FindOptions { - name_pattern: None, - type_filter: None, - max_depth: None, - min_depth: None, - printf_format: None, - }; +/// Parse find arguments into search paths and options. +/// Returns (paths, opts) or an error ExecResult. +#[allow(clippy::result_large_err)] +fn parse_find_args(args: &[String]) -> std::result::Result<(Vec, FindOptions), ExecResult> { + let mut paths: Vec = Vec::new(); + let mut opts = FindOptions { + name_pattern: None, + type_filter: None, + max_depth: None, + min_depth: None, + printf_format: None, + exec_args: Vec::new(), + exec_batch: false, + }; - // Parse arguments - let mut i = 0; - while i < ctx.args.len() { - let arg = &ctx.args[i]; - match arg.as_str() { - "-name" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-name'\n".to_string(), - 1, - )); - } - opts.name_pattern = Some(ctx.args[i].clone()); + let mut i = 0; + while i < args.len() { + let arg = &args[i]; + match arg.as_str() { + "-name" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "find: missing argument to '-name'\n".to_string(), + 1, + )); } - "-type" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-type'\n".to_string(), - 1, - )); - } - let t = &ctx.args[i]; - match t.as_str() { - "f" | "d" | "l" => opts.type_filter = Some(t.chars().next().unwrap()), - _ => { - return Ok(ExecResult::err(format!("find: unknown type '{}'\n", t), 1)); - } - } + opts.name_pattern = Some(args[i].clone()); + } + "-type" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "find: missing argument to '-type'\n".to_string(), + 1, + )); } - "-maxdepth" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-maxdepth'\n".to_string(), - 1, - )); - } - match ctx.args[i].parse::() { - Ok(n) => opts.max_depth = Some(n), - Err(_) => { - return Ok(ExecResult::err( - format!("find: invalid maxdepth value '{}'\n", ctx.args[i]), - 1, - )); - } + let t = &args[i]; + match t.as_str() { + "f" | "d" | "l" => opts.type_filter = Some(t.chars().next().unwrap()), + _ => { + return Err(ExecResult::err(format!("find: unknown type '{}'\n", t), 1)); } } - "-mindepth" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-mindepth'\n".to_string(), + } + "-maxdepth" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "find: missing argument to '-maxdepth'\n".to_string(), + 1, + )); + } + match args[i].parse::() { + Ok(n) => opts.max_depth = Some(n), + Err(_) => { + return Err(ExecResult::err( + format!("find: invalid maxdepth value '{}'\n", args[i]), 1, )); } - match ctx.args[i].parse::() { - Ok(n) => opts.min_depth = Some(n), - Err(_) => { - return Ok(ExecResult::err( - format!("find: invalid mindepth value '{}'\n", ctx.args[i]), - 1, - )); - } - } } - "-print" | "-print0" => { - // Default action, ignore + } + "-mindepth" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "find: missing argument to '-mindepth'\n".to_string(), + 1, + )); } - "-printf" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-printf'\n".to_string(), + match args[i].parse::() { + Ok(n) => opts.min_depth = Some(n), + Err(_) => { + return Err(ExecResult::err( + format!("find: invalid mindepth value '{}'\n", args[i]), 1, )); } - opts.printf_format = Some(ctx.args[i].clone()); - } - "-exec" | "-execdir" => { - // -exec is handled at interpreter level (execute_find); - // skip args here for fallback path - i += 1; - while i < ctx.args.len() { - let a = &ctx.args[i]; - if a == ";" || a == "\\;" || a == "+" { - break; - } - i += 1; - } } - "-not" | "!" => { - // Negation - skip (not fully supported) - } - s if s.starts_with('-') => { - return Ok(ExecResult::err( - format!("find: unknown predicate '{}'\n", s), + } + "-print" | "-print0" => { + // Default action, ignore + } + "-printf" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "find: missing argument to '-printf'\n".to_string(), 1, )); } - _ => { - paths.push(arg.clone()); + opts.printf_format = Some(args[i].clone()); + } + "-exec" | "-execdir" => { + i += 1; + while i < args.len() { + let a = &args[i]; + if a == ";" || a == "\\;" { + break; + } + if a == "+" { + opts.exec_batch = true; + break; + } + opts.exec_args.push(a.clone()); + i += 1; } } - i += 1; + "-not" | "!" => { + // Negation - skip (not fully supported) + } + s if s.starts_with('-') => { + return Err(ExecResult::err( + format!("find: unknown predicate '{}'\n", s), + 1, + )); + } + _ => { + paths.push(arg.clone()); + } } + i += 1; + } - // Default to current directory - if paths.is_empty() { - paths.push(".".to_string()); + if paths.is_empty() { + paths.push(".".to_string()); + } + + Ok((paths, opts)) +} + +/// Collect matched paths for find, without -exec output. +async fn collect_find_paths( + ctx: &Context<'_>, + search_paths: &[String], + opts: &FindOptions, +) -> Result> { + let mut matched: Vec = Vec::new(); + // Reuse find_recursive but with a temporary output buffer + let temp_opts = FindOptions { + name_pattern: opts.name_pattern.clone(), + type_filter: opts.type_filter, + max_depth: opts.max_depth, + min_depth: opts.min_depth, + printf_format: None, // Don't format, just collect paths + exec_args: Vec::new(), + exec_batch: false, + }; + let mut output = String::new(); + for path_str in search_paths { + let path = resolve_path(ctx.cwd, path_str); + if !ctx.fs.exists(&path).await.unwrap_or(false) { + // Skip non-existent paths in collection phase + continue; } + find_recursive(ctx, &path, path_str, &temp_opts, 0, &mut output).await?; + } + // Parse the output back into paths (each line is a path) + for line in output.lines() { + if !line.is_empty() { + matched.push(line.to_string()); + } + } + Ok(matched) +} - let mut output = String::new(); +/// Build exec sub-commands from matched paths and exec_args template. +fn build_find_exec_commands( + exec_args: &[String], + matched_paths: &[String], + batch: bool, +) -> Vec { + if exec_args.is_empty() || matched_paths.is_empty() { + return Vec::new(); + } + + if batch { + // Batch mode: -exec cmd {} + + // Replace {} with all paths at once + let cmd_args: Vec = exec_args + .iter() + .flat_map(|arg| { + if arg == "{}" { + matched_paths.to_vec() + } else { + vec![arg.clone()] + } + }) + .collect(); - for path_str in &paths { - let path = resolve_path(ctx.cwd, path_str); + if cmd_args.is_empty() { + return Vec::new(); + } + + vec![SubCommand { + name: cmd_args[0].clone(), + args: cmd_args[1..].to_vec(), + stdin: None, + }] + } else { + // Per-file mode: -exec cmd {} \; + matched_paths + .iter() + .map(|found_path| { + let cmd_args: Vec = exec_args + .iter() + .map(|arg| arg.replace("{}", found_path)) + .collect(); + + SubCommand { + name: cmd_args[0].clone(), + args: cmd_args[1..].to_vec(), + stdin: None, + } + }) + .collect() + } +} +#[async_trait] +impl Builtin for Find { + async fn execute(&self, ctx: Context<'_>) -> Result { + let (search_paths, opts) = match parse_find_args(ctx.args) { + Ok(v) => v, + Err(e) => return Ok(e), + }; + + // Check paths exist + for path_str in &search_paths { + let path = resolve_path(ctx.cwd, path_str); if !ctx.fs.exists(&path).await.unwrap_or(false) { return Ok(ExecResult::err( format!("find: '{}': No such file or directory\n", path_str), 1, )); } + } + let mut output = String::new(); + for path_str in &search_paths { + let path = resolve_path(ctx.cwd, path_str); find_recursive(&ctx, &path, path_str, &opts, 0, &mut output).await?; } Ok(ExecResult::ok(output)) } + + async fn execution_plan(&self, ctx: &Context<'_>) -> Result> { + let (search_paths, opts) = match parse_find_args(ctx.args) { + Ok(v) => v, + Err(_) => return Ok(None), // Let execute() handle errors + }; + + // Only return a plan when -exec is present + if opts.exec_args.is_empty() { + return Ok(None); + } + + // Check paths exist + for path_str in &search_paths { + let path = resolve_path(ctx.cwd, path_str); + if !ctx.fs.exists(&path).await.unwrap_or(false) { + return Ok(None); // Let execute() handle errors + } + } + + // Collect matched paths + let matched_paths = collect_find_paths(ctx, &search_paths, &opts).await?; + if matched_paths.is_empty() { + return Ok(None); + } + + let commands = build_find_exec_commands(&opts.exec_args, &matched_paths, opts.exec_batch); + if commands.is_empty() { + return Ok(None); + } + + Ok(Some(ExecutionPlan::Batch { commands })) + } } fn find_recursive<'a>( diff --git a/crates/bashkit/src/builtins/mod.rs b/crates/bashkit/src/builtins/mod.rs index 8fabd9af..4750524d 100644 --- a/crates/bashkit/src/builtins/mod.rs +++ b/crates/bashkit/src/builtins/mod.rs @@ -89,7 +89,7 @@ mod system; mod template; mod test; mod textrev; -mod timeout; +pub(crate) mod timeout; mod tomlq; mod tree; mod vars; @@ -202,6 +202,43 @@ use crate::interpreter::ExecResult; // Re-export for use by builtins pub use crate::interpreter::BuiltinSideEffect; +/// A sub-command that a builtin wants the interpreter to execute. +/// +/// Builtins like `timeout`, `xargs`, and `find -exec` need to execute +/// other commands. They return an [`ExecutionPlan`] describing what to +/// run, and the interpreter handles actual execution. +#[derive(Debug, Clone)] +pub struct SubCommand { + /// Command name (e.g. "echo", "rm"). + pub name: String, + /// Command arguments. + pub args: Vec, + /// Optional stdin to pipe into the command. + pub stdin: Option, +} + +/// Execution plan returned by builtins that need to run sub-commands. +/// +/// Instead of executing commands directly (which would require interpreter +/// access), builtins return a plan that the interpreter fulfills. +#[derive(Debug)] +pub enum ExecutionPlan { + /// Run a single command with a timeout. + Timeout { + /// Maximum duration before killing the command. + duration: std::time::Duration, + /// Whether to preserve the command's exit status on timeout. + preserve_status: bool, + /// The command to execute. + command: SubCommand, + }, + /// Run a sequence of commands, collecting their output. + Batch { + /// Commands to execute in order. + commands: Vec, + }, +} + /// Resolve a path relative to the current working directory. /// /// If the path is absolute, returns it unchanged. @@ -397,6 +434,20 @@ pub trait Builtin: Send + Sync { /// * `Err(Error)` - Fatal error that should abort execution async fn execute(&self, ctx: Context<'_>) -> Result; + /// Return an execution plan for sub-command execution. + /// + /// Builtins that need to execute other commands (e.g. `timeout`, `xargs`, + /// `find -exec`) override this to return an [`ExecutionPlan`]. The interpreter + /// fulfills the plan by executing the sub-commands and returning results. + /// + /// When this returns `Some(plan)`, the interpreter ignores the `execute()` + /// result and instead runs the plan. When `None`, normal `execute()` is used. + /// + /// The default implementation returns `Ok(None)`. + async fn execution_plan(&self, _ctx: &Context<'_>) -> Result> { + Ok(None) + } + /// Optional short hint for LLM system prompts. /// /// Return a concise one-line description of capabilities and limitations. @@ -421,6 +472,10 @@ impl Builtin for std::sync::Arc { (**self).execute(ctx).await } + async fn execution_plan(&self, ctx: &Context<'_>) -> Result> { + (**self).execution_plan(ctx).await + } + fn llm_hint(&self) -> Option<&'static str> { (**self).llm_hint() } diff --git a/crates/bashkit/src/builtins/pipeline.rs b/crates/bashkit/src/builtins/pipeline.rs index a8b88250..b4ad80bd 100644 --- a/crates/bashkit/src/builtins/pipeline.rs +++ b/crates/bashkit/src/builtins/pipeline.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; -use super::{Builtin, Context, resolve_path}; +use super::{Builtin, Context, ExecutionPlan, SubCommand, resolve_path}; use crate::error::Result; use crate::interpreter::ExecResult; @@ -15,130 +15,191 @@ use crate::interpreter::ExecResult; /// -n MAX-ARGS Use at most MAX-ARGS arguments per command /// -d DELIM Use DELIM as delimiter instead of whitespace /// -0 Use NUL as delimiter (same as -d '\0') -/// -/// Note: xargs is intercepted at the interpreter level for actual command -/// execution. This builtin fallback only handles option parsing/validation. pub struct Xargs; -#[async_trait] -impl Builtin for Xargs { - async fn execute(&self, ctx: Context<'_>) -> Result { - let mut replace_str: Option = None; - let mut max_args: Option = None; - let mut delimiter: Option = None; - let mut command: Vec = Vec::new(); +/// Parsed xargs options. +struct XargsOptions { + replace_str: Option, + max_args: Option, + delimiter: Option, + command: Vec, +} - // Parse arguments - let mut i = 0; - while i < ctx.args.len() { - let arg = &ctx.args[i]; - match arg.as_str() { - "-I" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'I'\n".to_string(), - 1, - )); - } - replace_str = Some(ctx.args[i].clone()); - max_args = Some(1); // -I implies -n 1 +/// Parse xargs arguments, returning options or an error ExecResult. +#[allow(clippy::result_large_err)] +fn parse_xargs_args(args: &[String]) -> std::result::Result { + let mut replace_str: Option = None; + let mut max_args: Option = None; + let mut delimiter: Option = None; + let mut command: Vec = Vec::new(); + + let mut i = 0; + while i < args.len() { + let arg = &args[i]; + match arg.as_str() { + "-I" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "xargs: option requires an argument -- 'I'\n".to_string(), + 1, + )); } - "-n" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'n'\n".to_string(), - 1, - )); - } - match ctx.args[i].parse::() { - Ok(n) if n > 0 => max_args = Some(n), - _ => { - return Ok(ExecResult::err( - format!("xargs: invalid number: '{}'\n", ctx.args[i]), - 1, - )); - } - } + replace_str = Some(args[i].clone()); + max_args = Some(1); // -I implies -n 1 + } + "-n" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "xargs: option requires an argument -- 'n'\n".to_string(), + 1, + )); } - "-d" => { - i += 1; - if i >= ctx.args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'd'\n".to_string(), + match args[i].parse::() { + Ok(n) if n > 0 => max_args = Some(n), + _ => { + return Err(ExecResult::err( + format!("xargs: invalid number: '{}'\n", args[i]), 1, )); } - delimiter = ctx.args[i].chars().next(); } - "-0" => { - delimiter = Some('\0'); - } - s if s.starts_with('-') && s != "-" => { - return Ok(ExecResult::err( - format!("xargs: invalid option -- '{}'\n", &s[1..]), + } + "-d" => { + i += 1; + if i >= args.len() { + return Err(ExecResult::err( + "xargs: option requires an argument -- 'd'\n".to_string(), 1, )); } - _ => { - // Rest is the command - command.extend(ctx.args[i..].iter().cloned()); - break; - } + delimiter = args[i].chars().next(); + } + "-0" => { + delimiter = Some('\0'); + } + s if s.starts_with('-') && s != "-" => { + return Err(ExecResult::err( + format!("xargs: invalid option -- '{}'\n", &s[1..]), + 1, + )); + } + _ => { + command.extend(args[i..].iter().cloned()); + break; } - i += 1; } + i += 1; + } - // Default command is echo - if command.is_empty() { - command.push("echo".to_string()); - } + if command.is_empty() { + command.push("echo".to_string()); + } + + Ok(XargsOptions { + replace_str, + max_args, + delimiter, + command, + }) +} + +/// Build the list of sub-commands from parsed options and stdin input. +fn build_xargs_commands(opts: &XargsOptions, input: &str) -> Vec { + if input.is_empty() { + return Vec::new(); + } + + let items: Vec<&str> = if let Some(delim) = opts.delimiter { + input.split(delim).filter(|s| !s.is_empty()).collect() + } else { + input.split_whitespace().collect() + }; + + if items.is_empty() { + return Vec::new(); + } + + let chunk_size = opts.max_args.unwrap_or(items.len()); + let chunks: Vec> = items.chunks(chunk_size).map(|c| c.to_vec()).collect(); + + chunks + .into_iter() + .map(|chunk| { + let cmd_args: Vec = if let Some(ref repl) = opts.replace_str { + let item = chunk.first().unwrap_or(&""); + opts.command + .iter() + .map(|arg| arg.replace(repl, item)) + .collect() + } else { + let mut full = opts.command.clone(); + full.extend(chunk.iter().map(|s| s.to_string())); + full + }; + + let name = cmd_args[0].clone(); + let args = cmd_args[1..].to_vec(); + SubCommand { + name, + args, + stdin: None, + } + }) + .collect() +} + +#[async_trait] +impl Builtin for Xargs { + async fn execute(&self, ctx: Context<'_>) -> Result { + // Validate arguments and return error for invalid input. + // When no executor is available, output what commands would be run. + let opts = match parse_xargs_args(ctx.args) { + Ok(opts) => opts, + Err(e) => return Ok(e), + }; - // Read input let input = ctx.stdin.unwrap_or(""); if input.is_empty() { return Ok(ExecResult::ok(String::new())); } - // Split input by delimiter - let items: Vec<&str> = if let Some(delim) = delimiter { - input.split(delim).filter(|s| !s.is_empty()).collect() - } else { - input.split_whitespace().collect() - }; - - if items.is_empty() { + let commands = build_xargs_commands(&opts, input); + if commands.is_empty() { return Ok(ExecResult::ok(String::new())); } + // Fallback: output what would be run (for standalone builtin context) let mut output = String::new(); + for cmd in &commands { + output.push_str(&cmd.name); + for arg in &cmd.args { + output.push(' '); + output.push_str(arg); + } + output.push('\n'); + } + Ok(ExecResult::ok(output)) + } - // Group items based on max_args - let chunk_size = max_args.unwrap_or(items.len()); - let chunks: Vec> = items.chunks(chunk_size).map(|c| c.to_vec()).collect(); - - for chunk in chunks { - if let Some(ref repl) = replace_str { - // With -I, substitute REPLACE string - let item = chunk.first().unwrap_or(&""); - let cmd: Vec = command.iter().map(|arg| arg.replace(repl, item)).collect(); + async fn execution_plan(&self, ctx: &Context<'_>) -> Result> { + let opts = match parse_xargs_args(ctx.args) { + Ok(opts) => opts, + Err(_) => return Ok(None), // Let execute() handle the error + }; - // Output the command that would be run - output.push_str(&cmd.join(" ")); - output.push('\n'); - } else { - // Append items as arguments - let mut cmd = command.clone(); - cmd.extend(chunk.iter().map(|s| s.to_string())); + let input = ctx.stdin.unwrap_or(""); + if input.is_empty() { + return Ok(None); // Let execute() handle empty input + } - // Output the command that would be run - output.push_str(&cmd.join(" ")); - output.push('\n'); - } + let commands = build_xargs_commands(&opts, input); + if commands.is_empty() { + return Ok(None); } - Ok(ExecResult::ok(output)) + Ok(Some(ExecutionPlan::Batch { commands })) } } @@ -178,10 +239,8 @@ impl Builtin for Tee { let path = resolve_path(ctx.cwd, file); if append { - // Append to existing file or create new one ctx.fs.append_file(&path, input.as_bytes()).await?; } else { - // Overwrite file ctx.fs.write_file(&path, input.as_bytes()).await?; } } @@ -208,7 +267,6 @@ impl Builtin for Watch { let mut _interval: f64 = 2.0; let mut command_start: Option = None; - // Parse arguments let mut i = 0; while i < ctx.args.len() { let arg = &ctx.args[i]; @@ -231,7 +289,6 @@ impl Builtin for Watch { } } else if arg.starts_with('-') && arg != "-" { // Skip other options for compatibility - // -d, -t, -b, -e, -g are common watch options, ignore them } else { command_start = Some(i); break; @@ -249,8 +306,6 @@ impl Builtin for Watch { } }; - // In virtual mode, we just display what the command would be - // A real implementation would need interpreter support to execute commands let command: Vec<_> = ctx.args[start..].iter().collect(); let output = format!( "Every {:.1}s: {}\n\n(watch: continuous execution not supported in virtual mode)\n", @@ -467,6 +522,68 @@ mod tests { assert!(result.stderr.contains("invalid option")); } + #[tokio::test] + async fn test_xargs_plan_basic() { + let (fs, mut cwd, mut variables) = create_test_ctx().await; + let env = HashMap::new(); + + let args = vec!["rm".to_string()]; + let ctx = Context { + args: &args, + env: &env, + variables: &mut variables, + cwd: &mut cwd, + fs: fs.clone(), + stdin: Some("file1 file2"), + #[cfg(feature = "http_client")] + http_client: None, + #[cfg(feature = "git")] + git_client: None, + }; + + let plan = Xargs.execution_plan(&ctx).await.unwrap(); + match plan { + Some(ExecutionPlan::Batch { commands }) => { + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].name, "rm"); + assert_eq!(commands[0].args, vec!["file1", "file2"]); + } + _ => panic!("expected Batch plan"), + } + } + + #[tokio::test] + async fn test_xargs_plan_n_option() { + let (fs, mut cwd, mut variables) = create_test_ctx().await; + let env = HashMap::new(); + + let args = vec!["-n".to_string(), "1".to_string(), "echo".to_string()]; + let ctx = Context { + args: &args, + env: &env, + variables: &mut variables, + cwd: &mut cwd, + fs: fs.clone(), + stdin: Some("a b c"), + #[cfg(feature = "http_client")] + http_client: None, + #[cfg(feature = "git")] + git_client: None, + }; + + let plan = Xargs.execution_plan(&ctx).await.unwrap(); + match plan { + Some(ExecutionPlan::Batch { commands }) => { + assert_eq!(commands.len(), 3); + assert_eq!(commands[0].name, "echo"); + assert_eq!(commands[0].args, vec!["a"]); + assert_eq!(commands[1].args, vec!["b"]); + assert_eq!(commands[2].args, vec!["c"]); + } + _ => panic!("expected Batch plan"), + } + } + // ==================== tee tests ==================== #[tokio::test] @@ -492,7 +609,6 @@ mod tests { assert_eq!(result.exit_code, 0); assert_eq!(result.stdout, "Hello, world!"); - // Verify file was written let content = fs.read_file(&cwd.join("output.txt")).await.unwrap(); assert_eq!(content, b"Hello, world!"); } @@ -520,7 +636,6 @@ mod tests { assert_eq!(result.exit_code, 0); assert_eq!(result.stdout, "content"); - // Verify both files were written let content1 = fs.read_file(&cwd.join("file1.txt")).await.unwrap(); let content2 = fs.read_file(&cwd.join("file2.txt")).await.unwrap(); assert_eq!(content1, b"content"); @@ -532,7 +647,6 @@ mod tests { let (fs, mut cwd, mut variables) = create_test_ctx().await; let env = HashMap::new(); - // Create initial file fs.write_file(&cwd.join("output.txt"), b"initial\n") .await .unwrap(); @@ -554,7 +668,6 @@ mod tests { let result = Tee.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); - // Verify file was appended let content = fs.read_file(&cwd.join("output.txt")).await.unwrap(); assert_eq!(content, b"initial\nappended"); } diff --git a/crates/bashkit/src/builtins/timeout.rs b/crates/bashkit/src/builtins/timeout.rs index e703f843..8febc62f 100644 --- a/crates/bashkit/src/builtins/timeout.rs +++ b/crates/bashkit/src/builtins/timeout.rs @@ -1,11 +1,12 @@ //! Timeout builtin - run command with time limit //! //! Executes a command with a specified timeout duration. +//! Returns an [`ExecutionPlan::Timeout`] for the interpreter to fulfill. use async_trait::async_trait; use std::time::Duration; -use super::{Builtin, Context}; +use super::{Builtin, Context, ExecutionPlan, SubCommand}; use crate::error::Result; use crate::interpreter::ExecResult; @@ -18,6 +19,7 @@ use crate::interpreter::ExecResult; /// Ns - N seconds /// Nm - N minutes /// Nh - N hours +/// Nd - N days /// /// Options: /// -k DURATION - Send KILL signal after DURATION if command still running @@ -38,125 +40,134 @@ pub struct Timeout; const MAX_TIMEOUT_SECONDS: u64 = 300; // 5 minutes max -/// Parse a duration string like "30", "30s", "5m", "1h" -fn parse_duration(s: &str) -> Option { +/// Parse a duration string like "30", "30s", "5m", "1h", "1d" +pub(crate) fn parse_duration(s: &str) -> Option { let s = s.trim(); if s.is_empty() { return None; } - // Check for suffix let (num_str, multiplier) = if let Some(stripped) = s.strip_suffix('s') { (stripped, 1u64) } else if let Some(stripped) = s.strip_suffix('m') { (stripped, 60u64) } else if let Some(stripped) = s.strip_suffix('h') { (stripped, 3600u64) + } else if let Some(stripped) = s.strip_suffix('d') { + (stripped, 86400u64) } else { (s, 1u64) // Default to seconds }; - // Parse the number (support decimals) let seconds: f64 = num_str.parse().ok()?; if seconds < 0.0 { return None; } - let total_seconds = (seconds * multiplier as f64) as u64; - - // Cap at max timeout - let capped = total_seconds.min(MAX_TIMEOUT_SECONDS); - - Some(Duration::from_secs(capped)) + let total_secs_f64 = seconds * multiplier as f64; + // Cap at max while preserving subsecond precision + let max = Duration::from_secs(MAX_TIMEOUT_SECONDS); + let d = Duration::from_secs_f64(total_secs_f64); + Some(if d > max { max } else { d }) } -#[async_trait] -impl Builtin for Timeout { - async fn execute(&self, ctx: Context<'_>) -> Result { - if ctx.args.is_empty() { - return Ok(ExecResult::err( - "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), - 125, - )); - } +/// Parse timeout arguments, returning (preserve_status, duration, cmd_name, cmd_args) +/// or an error ExecResult. +#[allow(clippy::result_large_err)] +fn parse_timeout_args( + args: &[String], +) -> std::result::Result<(bool, Duration, String, Vec), ExecResult> { + if args.is_empty() { + return Err(ExecResult::err( + "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), + 125, + )); + } - // Parse options - let mut preserve_status = false; - let mut duration_idx = 0; - - for (i, arg) in ctx.args.iter().enumerate() { - match arg.as_str() { - "--preserve-status" => { - preserve_status = true; - duration_idx = i + 1; - } - "-k" | "-s" => { - // Skip the next argument (these options take a value) - duration_idx = i + 2; - } - _ if arg.starts_with('-') => { - // Skip unknown options - duration_idx = i + 1; - } - _ => { - duration_idx = i; - break; - } + let mut preserve_status = false; + let mut arg_idx = 0; + + while arg_idx < args.len() { + let arg = &args[arg_idx]; + match arg.as_str() { + "--preserve-status" => { + preserve_status = true; + arg_idx += 1; + } + "-k" | "-s" => { + // These options take a value, skip it + arg_idx += 2; + } + s if s.starts_with('-') && !s.chars().nth(1).is_some_and(|c| c.is_ascii_digit()) => { + arg_idx += 1; } + _ => break, // Found duration } + } + + if arg_idx >= args.len() { + return Err(ExecResult::err( + "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), + 125, + )); + } - if duration_idx >= ctx.args.len() { - return Ok(ExecResult::err( - "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), + let duration_str = &args[arg_idx]; + let duration = match parse_duration(duration_str) { + Some(d) => d, + None => { + return Err(ExecResult::err( + format!("timeout: invalid time interval '{}'\n", duration_str), 125, )); } + }; - // Parse duration - let duration = match parse_duration(&ctx.args[duration_idx]) { - Some(d) => d, - None => { - return Ok(ExecResult::err( - format!( - "timeout: invalid time interval '{}'\n", - ctx.args[duration_idx] - ), - 125, - )); - } - }; + arg_idx += 1; - // Get command and args - let cmd_idx = duration_idx + 1; - if cmd_idx >= ctx.args.len() { - return Ok(ExecResult::err( - "timeout: missing command\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), - 125, - )); + if arg_idx >= args.len() { + return Err(ExecResult::err( + "timeout: missing command\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), + 125, + )); + } + + let cmd_name = args[arg_idx].clone(); + let cmd_args = args[arg_idx + 1..].to_vec(); + + Ok((preserve_status, duration, cmd_name, cmd_args)) +} + +#[async_trait] +impl Builtin for Timeout { + async fn execute(&self, ctx: Context<'_>) -> Result { + // Validate arguments and return error for invalid input. + // Actual command execution is handled by execution_plan(). + match parse_timeout_args(ctx.args) { + Err(e) => Ok(e), + Ok(_) => { + // Valid args but no executor available (standalone builtin context). + // This shouldn't normally happen since the interpreter uses execution_plan(). + Ok(ExecResult::ok(String::new())) + } } + } - let command = &ctx.args[cmd_idx]; - let command_args: Vec = ctx.args[cmd_idx + 1..].to_vec(); - - // Note: In the current Bashkit architecture, we can't easily execute - // arbitrary commands from within a builtin. The timeout would need to - // be implemented at the interpreter level to wrap command execution. - // - // For now, we return an informative message about the limitation. - // A full implementation would require interpreter-level changes. - - let _ = (duration, preserve_status, command, command_args); - - Ok(ExecResult::err( - format!( - "timeout: command execution not available from builtin context\n\ - Note: timeout requires interpreter-level integration.\n\ - Requested: timeout {:?} {} ...\n\ - Consider using execution limits instead.\n", - duration, command - ), - 125, - )) + async fn execution_plan(&self, ctx: &Context<'_>) -> Result> { + match parse_timeout_args(ctx.args) { + Err(_) => Ok(None), // Let execute() handle the error + Ok((preserve_status, duration, cmd_name, cmd_args)) => { + Ok(Some(ExecutionPlan::Timeout { + duration, + preserve_status, + command: SubCommand { + name: cmd_name, + args: cmd_args, + stdin: ctx.stdin.map(|s| s.to_string()), + }, + })) + } + } } } @@ -193,6 +204,29 @@ mod tests { Timeout.execute(ctx).await.unwrap() } + async fn get_plan(args: &[&str], stdin: Option<&str>) -> Option { + let fs = Arc::new(InMemoryFs::new()); + let mut variables = HashMap::new(); + let env = HashMap::new(); + let mut cwd = PathBuf::from("/"); + + let args: Vec = args.iter().map(|s| s.to_string()).collect(); + let ctx = Context { + args: &args, + env: &env, + variables: &mut variables, + cwd: &mut cwd, + fs, + stdin, + #[cfg(feature = "http_client")] + http_client: None, + #[cfg(feature = "git")] + git_client: None, + }; + + Timeout.execution_plan(&ctx).await.unwrap() + } + #[test] fn test_parse_duration_seconds() { assert_eq!(parse_duration("30"), Some(Duration::from_secs(30))); @@ -212,10 +246,16 @@ mod tests { assert_eq!(parse_duration("1h"), Some(Duration::from_secs(300))); } + #[test] + fn test_parse_duration_days() { + // Capped at MAX_TIMEOUT_SECONDS (300) + assert_eq!(parse_duration("1d"), Some(Duration::from_secs(300))); + } + #[test] fn test_parse_duration_decimal() { - assert_eq!(parse_duration("1.5"), Some(Duration::from_secs(1))); - assert_eq!(parse_duration("0.5s"), Some(Duration::from_secs(0))); + let d = parse_duration("1.5").unwrap(); + assert!(d.as_secs_f64() > 1.4 && d.as_secs_f64() < 1.6); } #[test] @@ -247,10 +287,54 @@ mod tests { } #[tokio::test] - async fn test_timeout_with_command() { - let result = run_timeout(&["30", "echo", "hello"]).await; - // Currently returns stub error - assert_eq!(result.exit_code, 125); - assert!(result.stderr.contains("interpreter-level")); + async fn test_timeout_plan_basic() { + let plan = get_plan(&["30", "echo", "hello"], None).await; + match plan { + Some(ExecutionPlan::Timeout { + duration, + preserve_status, + command, + }) => { + assert_eq!(duration, Duration::from_secs(30)); + assert!(!preserve_status); + assert_eq!(command.name, "echo"); + assert_eq!(command.args, vec!["hello"]); + assert!(command.stdin.is_none()); + } + _ => panic!("expected Timeout plan"), + } + } + + #[tokio::test] + async fn test_timeout_plan_preserve_status() { + let plan = get_plan(&["--preserve-status", "5", "sleep", "10"], None).await; + match plan { + Some(ExecutionPlan::Timeout { + preserve_status, .. + }) => { + assert!(preserve_status); + } + _ => panic!("expected Timeout plan"), + } + } + + #[tokio::test] + async fn test_timeout_plan_with_stdin() { + let plan = get_plan(&["5", "cat"], Some("hello\n")).await; + match plan { + Some(ExecutionPlan::Timeout { command, .. }) => { + assert_eq!(command.stdin.as_deref(), Some("hello\n")); + } + _ => panic!("expected Timeout plan"), + } + } + + #[tokio::test] + async fn test_timeout_plan_invalid_returns_none() { + let plan = get_plan(&[], None).await; + assert!(plan.is_none()); + + let plan = get_plan(&["abc", "echo"], None).await; + assert!(plan.is_none()); } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 0cb167a1..ac49e897 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -2377,835 +2377,6 @@ impl Interpreter { None } - /// Execute a timeout command - run command with time limit - /// - /// Usage: timeout [OPTIONS] DURATION COMMAND [ARGS...] - /// - /// Options: - /// --preserve-status Exit with command's status even on timeout - /// -k DURATION Kill signal timeout (ignored - always terminates) - /// -s SIGNAL Signal to send (ignored) - /// - /// Exit codes: - /// 124 - Command timed out - /// 125 - Timeout itself failed (bad arguments) - /// Otherwise, exit status of command - async fn execute_timeout( - &mut self, - args: &[String], - stdin: Option, - redirects: &[Redirect], - ) -> Result { - use std::time::Duration; - use tokio::time::timeout; - - const MAX_TIMEOUT_SECONDS: u64 = 300; // 5 minutes max for safety - - if args.is_empty() { - return Ok(ExecResult::err( - "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), - 125, - )); - } - - // Parse options and find duration/command - let mut preserve_status = false; - let mut arg_idx = 0; - - while arg_idx < args.len() { - let arg = &args[arg_idx]; - match arg.as_str() { - "--preserve-status" => { - preserve_status = true; - arg_idx += 1; - } - "-k" | "-s" => { - // These options take a value, skip it - arg_idx += 2; - } - s if s.starts_with('-') - && !s.chars().nth(1).is_some_and(|c| c.is_ascii_digit()) => - { - // Unknown option, skip - arg_idx += 1; - } - _ => break, // Found duration - } - } - - if arg_idx >= args.len() { - return Ok(ExecResult::err( - "timeout: missing operand\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), - 125, - )); - } - - // Parse duration - let duration_str = &args[arg_idx]; - let max_duration = Duration::from_secs(MAX_TIMEOUT_SECONDS); - let duration = match Self::parse_timeout_duration(duration_str) { - Some(d) => { - // Cap at max while preserving subsecond precision - if d > max_duration { max_duration } else { d } - } - None => { - return Ok(ExecResult::err( - format!("timeout: invalid time interval '{}'\n", duration_str), - 125, - )); - } - }; - - arg_idx += 1; - - if arg_idx >= args.len() { - return Ok(ExecResult::err( - "timeout: missing command\nUsage: timeout DURATION COMMAND [ARGS...]\n".to_string(), - 125, - )); - } - - // Build the inner command - let cmd_name = &args[arg_idx]; - let cmd_args: Vec = args[arg_idx + 1..].to_vec(); - - // If we have stdin from a pipeline, pass it to the inner command via here-string - let inner_redirects = if let Some(ref stdin_data) = stdin { - vec![Redirect { - fd: None, - kind: RedirectKind::HereString, - target: Word::literal(stdin_data.trim_end_matches('\n').to_string()), - }] - } else { - Vec::new() - }; - - // Create a SimpleCommand for the inner command - let inner_cmd = Command::Simple(SimpleCommand { - name: Word::literal(cmd_name.clone()), - args: cmd_args.iter().map(|s| Word::literal(s.clone())).collect(), - redirects: inner_redirects, - assignments: Vec::new(), - span: Span::new(), - }); - - // Execute with timeout using execute_command (which handles recursion via Box::pin) - let exec_future = self.execute_command(&inner_cmd); - let result = match timeout(duration, exec_future).await { - Ok(Ok(result)) => result, - Ok(Err(e)) => return Err(e), - Err(_) => { - // Timeout expired - if preserve_status { - // Return the timeout exit code but preserve-status means... - // actually in bash --preserve-status makes timeout return - // the command's exit status, but if it times out, there's no status - // so it still returns 124 - ExecResult::err(String::new(), 124) - } else { - ExecResult::err(String::new(), 124) - } - } - }; - - // Apply output redirections - self.apply_redirections(result, redirects).await - } - - /// Parse a timeout duration string like "30", "30s", "5m", "1h" - fn parse_timeout_duration(s: &str) -> Option { - use std::time::Duration; - - let s = s.trim(); - if s.is_empty() { - return None; - } - - // Check for suffix - let (num_str, multiplier) = if let Some(stripped) = s.strip_suffix('s') { - (stripped, 1u64) - } else if let Some(stripped) = s.strip_suffix('m') { - (stripped, 60u64) - } else if let Some(stripped) = s.strip_suffix('h') { - (stripped, 3600u64) - } else if let Some(stripped) = s.strip_suffix('d') { - (stripped, 86400u64) - } else { - (s, 1u64) // Default to seconds - }; - - // Parse the number (support decimals) - let seconds: f64 = num_str.parse().ok()?; - if seconds < 0.0 { - return None; - } - - let total_secs_f64 = seconds * multiplier as f64; - Some(Duration::from_secs_f64(total_secs_f64)) - } - - /// Execute `xargs` - build and execute command lines from stdin. - /// - /// Parses xargs options, splits stdin into arguments, and executes the - /// target command via the interpreter for each batch. - async fn execute_xargs( - &mut self, - args: &[String], - stdin: Option, - redirects: &[Redirect], - ) -> Result { - let mut replace_str: Option = None; - let mut max_args: Option = None; - let mut delimiter: Option = None; - let mut command: Vec = Vec::new(); - - // Parse xargs options - let mut i = 0; - while i < args.len() { - let arg = &args[i]; - match arg.as_str() { - "-I" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'I'\n".to_string(), - 1, - )); - } - replace_str = Some(args[i].clone()); - max_args = Some(1); // -I implies -n 1 - } - "-n" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'n'\n".to_string(), - 1, - )); - } - match args[i].parse::() { - Ok(n) if n > 0 => max_args = Some(n), - _ => { - return Ok(ExecResult::err( - format!("xargs: invalid number: '{}'\n", args[i]), - 1, - )); - } - } - } - "-d" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "xargs: option requires an argument -- 'd'\n".to_string(), - 1, - )); - } - delimiter = args[i].chars().next(); - } - "-0" => { - delimiter = Some('\0'); - } - s if s.starts_with('-') && s != "-" => { - return Ok(ExecResult::err( - format!("xargs: invalid option -- '{}'\n", &s[1..]), - 1, - )); - } - _ => { - // Rest is the command - command.extend(args[i..].iter().cloned()); - break; - } - } - i += 1; - } - - // Default command is echo - if command.is_empty() { - command.push("echo".to_string()); - } - - // Read input - let input = stdin.as_deref().unwrap_or(""); - if input.is_empty() { - let result = ExecResult::ok(String::new()); - return self.apply_redirections(result, redirects).await; - } - - // Split input by delimiter - let items: Vec<&str> = if let Some(delim) = delimiter { - input.split(delim).filter(|s| !s.is_empty()).collect() - } else { - input.split_whitespace().collect() - }; - - if items.is_empty() { - let result = ExecResult::ok(String::new()); - return self.apply_redirections(result, redirects).await; - } - - let mut combined_stdout = String::new(); - let mut combined_stderr = String::new(); - let mut last_exit_code = 0; - - // Group items based on max_args - let chunk_size = max_args.unwrap_or(items.len()); - let chunks: Vec> = items.chunks(chunk_size).map(|c| c.to_vec()).collect(); - - for chunk in chunks { - let cmd_args: Vec = if let Some(ref repl) = replace_str { - // With -I, substitute REPLACE string in all command args - let item = chunk.first().unwrap_or(&""); - command.iter().map(|arg| arg.replace(repl, item)).collect() - } else { - // Append chunk items as arguments after the command - let mut full = command.clone(); - full.extend(chunk.iter().map(|s| s.to_string())); - full - }; - - // Build a SimpleCommand and execute it through the interpreter - let cmd_name = cmd_args[0].clone(); - let cmd_rest: Vec = cmd_args[1..] - .iter() - .map(|s| Word::literal(s.clone())) - .collect(); - - let inner_cmd = Command::Simple(SimpleCommand { - name: Word::literal(cmd_name), - args: cmd_rest, - redirects: Vec::new(), - assignments: Vec::new(), - span: Span::new(), - }); - - let result = self.execute_command(&inner_cmd).await?; - combined_stdout.push_str(&result.stdout); - combined_stderr.push_str(&result.stderr); - last_exit_code = result.exit_code; - } - - let mut result = ExecResult { - stdout: combined_stdout, - stderr: combined_stderr, - exit_code: last_exit_code, - control_flow: ControlFlow::None, - ..Default::default() - }; - - result = self.apply_redirections(result, redirects).await?; - Ok(result) - } - - /// Execute `find` with `-exec` support. - /// - /// Intercepts find when -exec is present so commands can be executed - /// through the interpreter. Supports: - /// - `find PATH -exec cmd {} \;` (per-file execution) - /// - `find PATH -exec cmd {} +` (batch execution) - /// - All standard find options: -name, -type, -maxdepth - async fn execute_find( - &mut self, - args: &[String], - redirects: &[Redirect], - ) -> Result { - let mut search_paths: Vec = Vec::new(); - let mut name_pattern: Option = None; - let mut type_filter: Option = None; - let mut max_depth: Option = None; - let mut min_depth: Option = None; - let mut exec_args: Vec = Vec::new(); - let mut exec_batch = false; - let mut printf_format: Option = None; - - // Parse arguments - let mut i = 0; - while i < args.len() { - let arg = &args[i]; - match arg.as_str() { - "-name" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-name'\n".to_string(), - 1, - )); - } - name_pattern = Some(args[i].clone()); - } - "-type" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-type'\n".to_string(), - 1, - )); - } - match args[i].as_str() { - "f" | "d" | "l" => type_filter = Some(args[i].chars().next().unwrap()), - t => { - return Ok(ExecResult::err(format!("find: unknown type '{}'\n", t), 1)); - } - } - } - "-maxdepth" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-maxdepth'\n".to_string(), - 1, - )); - } - match args[i].parse::() { - Ok(n) => max_depth = Some(n), - Err(_) => { - return Ok(ExecResult::err( - format!("find: invalid maxdepth value '{}'\n", args[i]), - 1, - )); - } - } - } - "-mindepth" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-mindepth'\n".to_string(), - 1, - )); - } - match args[i].parse::() { - Ok(n) => min_depth = Some(n), - Err(_) => { - return Ok(ExecResult::err( - format!("find: invalid mindepth value '{}'\n", args[i]), - 1, - )); - } - } - } - "-print" | "-print0" => {} - "-printf" => { - i += 1; - if i >= args.len() { - return Ok(ExecResult::err( - "find: missing argument to '-printf'\n".to_string(), - 1, - )); - } - printf_format = Some(args[i].clone()); - } - "-exec" | "-execdir" => { - i += 1; - while i < args.len() { - let a = &args[i]; - if a == ";" || a == "\\;" { - break; - } - if a == "+" { - exec_batch = true; - break; - } - exec_args.push(a.clone()); - i += 1; - } - } - "-not" | "!" => {} - s if s.starts_with('-') => { - return Ok(ExecResult::err( - format!("find: unknown predicate '{}'\n", s), - 1, - )); - } - _ => { - search_paths.push(arg.clone()); - } - } - i += 1; - } - - if search_paths.is_empty() { - search_paths.push(".".to_string()); - } - - // Collect matching paths via recursive walk - let mut matched_paths: Vec = Vec::new(); - for path_str in &search_paths { - let path = self.resolve_path(path_str); - if !self.fs.exists(&path).await.unwrap_or(false) { - return Ok(ExecResult::err( - format!("find: '{}': No such file or directory\n", path_str), - 1, - )); - } - self.find_collect( - &path, - path_str, - &name_pattern, - type_filter, - max_depth, - 0, - &mut matched_paths, - ) - .await?; - } - - // Filter by mindepth - if let Some(min) = min_depth { - matched_paths.retain(|p| { - let depth = if search_paths.len() == 1 { - let base = &search_paths[0]; - if p == base { - 0 - } else { - let suffix = p.strip_prefix(base).unwrap_or(p); - let suffix = suffix.strip_prefix('/').unwrap_or(suffix); - if suffix.is_empty() { - 0 - } else { - suffix.matches('/').count() + 1 - } - } - } else { - 0 - }; - depth >= min - }); - } - - // Handle -printf output - if let Some(ref fmt) = printf_format { - let mut output = String::new(); - for found_path in &matched_paths { - let resolved = self.resolve_path(found_path); - let formatted = self - .find_printf_format(fmt, found_path, &resolved, &search_paths) - .await; - output.push_str(&formatted); - } - let result = ExecResult::ok(output); - return self.apply_redirections(result, redirects).await; - } - - // Execute commands for matched paths - if exec_args.is_empty() { - // No exec command parsed, just print - let output = - matched_paths.join("\n") + if matched_paths.is_empty() { "" } else { "\n" }; - let result = ExecResult::ok(output); - return self.apply_redirections(result, redirects).await; - } - - let mut combined_stdout = String::new(); - let mut combined_stderr = String::new(); - let mut last_exit_code = 0; - - if exec_batch { - // Batch mode: -exec cmd {} + - // Replace {} with all paths at once - let cmd_args: Vec = exec_args - .iter() - .flat_map(|arg| { - if arg == "{}" { - matched_paths.clone() - } else { - vec![arg.clone()] - } - }) - .collect(); - - if !cmd_args.is_empty() { - let cmd_name = cmd_args[0].clone(); - let cmd_rest: Vec = cmd_args[1..] - .iter() - .map(|s| Word::literal(s.clone())) - .collect(); - - let inner_cmd = Command::Simple(SimpleCommand { - name: Word::literal(cmd_name), - args: cmd_rest, - redirects: Vec::new(), - assignments: Vec::new(), - span: Span::new(), - }); - - let result = self.execute_command(&inner_cmd).await?; - combined_stdout.push_str(&result.stdout); - combined_stderr.push_str(&result.stderr); - last_exit_code = result.exit_code; - } - } else { - // Per-file mode: -exec cmd {} \; - for found_path in &matched_paths { - let cmd_args: Vec = exec_args - .iter() - .map(|arg| arg.replace("{}", found_path)) - .collect(); - - if cmd_args.is_empty() { - continue; - } - - let cmd_name = cmd_args[0].clone(); - let cmd_rest: Vec = cmd_args[1..] - .iter() - .map(|s| Word::literal(s.clone())) - .collect(); - - let inner_cmd = Command::Simple(SimpleCommand { - name: Word::literal(cmd_name), - args: cmd_rest, - redirects: Vec::new(), - assignments: Vec::new(), - span: Span::new(), - }); - - let result = self.execute_command(&inner_cmd).await?; - combined_stdout.push_str(&result.stdout); - combined_stderr.push_str(&result.stderr); - last_exit_code = result.exit_code; - } - } - - let mut result = ExecResult { - stdout: combined_stdout, - stderr: combined_stderr, - exit_code: last_exit_code, - control_flow: ControlFlow::None, - ..Default::default() - }; - - result = self.apply_redirections(result, redirects).await?; - Ok(result) - } - - /// Recursively collect paths matching find criteria. - /// - /// Helper for `execute_find`. Walks the filesystem tree and collects - /// display paths of entries matching name/type/depth filters. - #[allow(clippy::too_many_arguments)] - fn find_collect<'a>( - &'a self, - path: &'a Path, - display_path: &'a str, - name_pattern: &'a Option, - type_filter: Option, - max_depth: Option, - current_depth: usize, - results: &'a mut Vec, - ) -> std::pin::Pin> + Send + 'a>> { - Box::pin(async move { - use crate::builtins::glob_match; - - let metadata = self.fs.stat(path).await?; - let entry_name = Path::new(display_path) - .file_name() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| display_path.to_string()); - - let type_matches = match type_filter { - Some('f') => metadata.file_type.is_file(), - Some('d') => metadata.file_type.is_dir(), - Some('l') => metadata.file_type.is_symlink(), - _ => true, - }; - - let name_matches = match name_pattern { - Some(pattern) => glob_match(&entry_name, pattern), - None => true, - }; - - if type_matches && name_matches { - results.push(display_path.to_string()); - } - - if metadata.file_type.is_dir() { - if let Some(max) = max_depth - && current_depth >= max - { - return Ok(()); - } - - let entries = self.fs.read_dir(path).await?; - let mut sorted_entries = entries; - sorted_entries.sort_by(|a, b| a.name.cmp(&b.name)); - - for entry in sorted_entries { - let child_path = path.join(&entry.name); - let child_display = if display_path == "." { - format!("./{}", entry.name) - } else { - format!("{}/{}", display_path, entry.name) - }; - - self.find_collect( - &child_path, - &child_display, - name_pattern, - type_filter, - max_depth, - current_depth + 1, - results, - ) - .await?; - } - } - - Ok(()) - }) - } - - /// Format a single path using a `-printf` format string. - /// - /// Supported specifiers: `%f` (filename), `%p` (full path), `%P` (relative path), - /// `%s` (size), `%m` (octal mode), `%M` (symbolic mode), `%T@` (mtime epoch), - /// `%y` (type char), `%d` (depth). Escapes: `\n`, `\t`, `\0`, `\\`. - async fn find_printf_format( - &self, - fmt: &str, - display_path: &str, - resolved_path: &Path, - search_paths: &[String], - ) -> String { - let meta = self.fs.stat(resolved_path).await.ok(); - - let mut out = String::new(); - let chars: Vec = fmt.chars().collect(); - let mut i = 0; - while i < chars.len() { - match chars[i] { - '\\' => { - i += 1; - if i < chars.len() { - match chars[i] { - 'n' => out.push('\n'), - 't' => out.push('\t'), - '0' => out.push('\0'), - '\\' => out.push('\\'), - c => { - out.push('\\'); - out.push(c); - } - } - } - } - '%' => { - i += 1; - if i >= chars.len() { - out.push('%'); - continue; - } - match chars[i] { - 'f' => { - // Filename (basename) - let name = Path::new(display_path) - .file_name() - .map(|s| s.to_string_lossy().to_string()) - .unwrap_or_else(|| display_path.to_string()); - out.push_str(&name); - } - 'p' => { - out.push_str(display_path); - } - 'P' => { - // Path relative to search root (strip search path prefix) - let base = search_paths.first().map(|s| s.as_str()).unwrap_or("."); - let rel = display_path - .strip_prefix(base) - .unwrap_or(display_path) - .trim_start_matches('/'); - out.push_str(rel); - } - 's' => { - let size = meta.as_ref().map(|m| m.size).unwrap_or(0); - out.push_str(&size.to_string()); - } - 'm' => { - let mode = meta.as_ref().map(|m| m.mode).unwrap_or(0); - out.push_str(&format!("{:o}", mode & 0o7777)); - } - 'M' => { - // Symbolic mode like ls -l (e.g., -rw-r--r--) - let m = meta.as_ref(); - let ft = m.map(|m| &m.file_type); - let mode = m.map(|m| m.mode).unwrap_or(0); - let type_ch = match ft { - Some(ft) if ft.is_dir() => 'd', - Some(ft) if ft.is_symlink() => 'l', - _ => '-', - }; - out.push(type_ch); - for shift in [6, 3, 0] { - let bits = (mode >> shift) & 7; - out.push(if bits & 4 != 0 { 'r' } else { '-' }); - out.push(if bits & 2 != 0 { 'w' } else { '-' }); - out.push(if bits & 1 != 0 { 'x' } else { '-' }); - } - } - 'y' => { - let ch = match meta.as_ref().map(|m| &m.file_type) { - Some(ft) if ft.is_dir() => 'd', - Some(ft) if ft.is_symlink() => 'l', - Some(ft) if ft.is_file() => 'f', - _ => 'f', - }; - out.push(ch); - } - 'd' => { - // Depth relative to search root - let base = search_paths.first().map(|s| s.as_str()).unwrap_or("."); - let depth = if display_path == base { - 0 - } else { - let suffix = display_path - .strip_prefix(base) - .unwrap_or(display_path) - .trim_start_matches('/'); - if suffix.is_empty() { - 0 - } else { - suffix.matches('/').count() + 1 - } - }; - out.push_str(&depth.to_string()); - } - 'T' => { - // %T@ = mtime as seconds since epoch - i += 1; - if i < chars.len() && chars[i] == '@' { - let secs = meta - .as_ref() - .and_then(|m| { - m.modified - .duration_since(std::time::UNIX_EPOCH) - .ok() - .map(|d| d.as_secs()) - }) - .unwrap_or(0); - out.push_str(&secs.to_string()); - } else { - out.push_str("%T"); - // re-process current char - continue; - } - } - '%' => { - out.push('%'); - } - c => { - out.push('%'); - out.push(c); - } - } - } - c => out.push(c), - } - i += 1; - } - out - } - /// Execute `bash` or `sh` command - interpret scripts using this interpreter. /// /// Supports: @@ -4451,21 +3622,6 @@ impl Interpreter { return self.execute_history(&args, &command.redirects).await; } - // Handle `timeout` specially - needs interpreter-level command execution - if name == "timeout" { - return self.execute_timeout(&args, stdin, &command.redirects).await; - } - - // Handle `xargs` specially - needs interpreter-level command execution - if name == "xargs" { - return self.execute_xargs(&args, stdin, &command.redirects).await; - } - - // Handle `find -exec` specially - needs interpreter-level command execution - if name == "find" && args.iter().any(|a| a == "-exec" || a == "-execdir") { - return self.execute_find(&args, &command.redirects).await; - } - // Handle `bash` and `sh` specially - execute scripts using the interpreter if name == "bash" || name == "sh" { return self @@ -4560,6 +3716,45 @@ impl Interpreter { // Check for builtins if let Some(builtin) = self.builtins.get(name) { + // First, check if the builtin wants to return an execution plan + // (e.g. timeout, xargs, find -exec need the interpreter to run sub-commands) + { + let plan_ctx = builtins::Context { + args: &args, + env: &self.env, + variables: &mut self.variables, + cwd: &mut self.cwd, + fs: Arc::clone(&self.fs), + stdin: stdin.as_deref(), + #[cfg(feature = "http_client")] + http_client: self.http_client.as_ref(), + #[cfg(feature = "git")] + git_client: self.git_client.as_ref(), + }; + + let plan_result = AssertUnwindSafe(builtin.execution_plan(&plan_ctx)) + .catch_unwind() + .await; + + match plan_result { + Ok(Ok(Some(plan))) => { + let result = self.execute_builtin_plan(plan, &command.redirects).await?; + return Ok(result); + } + Ok(Ok(None)) => { + // No plan - fall through to normal execute() + } + Ok(Err(e)) => return Err(e), + Err(_panic) => { + let result = ExecResult::err( + format!("bash: {}: builtin failed unexpectedly\n", name), + 1, + ); + return self.apply_redirections(result, &command.redirects).await; + } + } + } + let ctx = builtins::Context { args: &args, env: &self.env, @@ -6551,6 +5746,90 @@ impl Interpreter { Ok(stdin) } + /// Execute an [`ExecutionPlan`] returned by a builtin's `execution_plan()` method. + /// + /// This is the interpreter hook that fulfills sub-command execution requests + /// from builtins like `timeout`, `xargs`, and `find -exec`. + async fn execute_builtin_plan( + &mut self, + plan: builtins::ExecutionPlan, + redirects: &[Redirect], + ) -> Result { + let result = match plan { + builtins::ExecutionPlan::Timeout { + duration, + preserve_status, + command, + } => { + use tokio::time::timeout; + + // Build inner command with optional stdin via here-string + let inner_redirects = if let Some(ref stdin_data) = command.stdin { + vec![Redirect { + fd: None, + kind: RedirectKind::HereString, + target: Word::literal(stdin_data.trim_end_matches('\n').to_string()), + }] + } else { + Vec::new() + }; + + let inner_cmd = Command::Simple(SimpleCommand { + name: Word::literal(command.name), + args: command + .args + .iter() + .map(|s| Word::literal(s.clone())) + .collect(), + redirects: inner_redirects, + assignments: Vec::new(), + span: Span::new(), + }); + + let exec_future = self.execute_command(&inner_cmd); + match timeout(duration, exec_future).await { + Ok(Ok(result)) => result, + Ok(Err(e)) => return Err(e), + Err(_) => { + // Timeout expired - exit code 124 + let _ = preserve_status; + ExecResult::err(String::new(), 124) + } + } + } + builtins::ExecutionPlan::Batch { commands } => { + let mut combined_stdout = String::new(); + let mut combined_stderr = String::new(); + let mut last_exit_code = 0; + + for cmd in commands { + let inner_cmd = Command::Simple(SimpleCommand { + name: Word::literal(cmd.name), + args: cmd.args.iter().map(|s| Word::literal(s.clone())).collect(), + redirects: Vec::new(), + assignments: Vec::new(), + span: Span::new(), + }); + + let result = self.execute_command(&inner_cmd).await?; + combined_stdout.push_str(&result.stdout); + combined_stderr.push_str(&result.stderr); + last_exit_code = result.exit_code; + } + + ExecResult { + stdout: combined_stdout, + stderr: combined_stderr, + exit_code: last_exit_code, + control_flow: ControlFlow::None, + ..Default::default() + } + } + }; + + self.apply_redirections(result, redirects).await + } + /// Process structured side effects from builtin execution. fn apply_builtin_side_effects(&mut self, result: &ExecResult) { for effect in &result.side_effects { @@ -9559,23 +8838,24 @@ mod tests { ); } - /// Test that parse_timeout_duration preserves subsecond precision + /// Test that parse_duration preserves subsecond precision #[test] fn test_parse_timeout_duration_subsecond() { + use crate::builtins::timeout::parse_duration; use std::time::Duration; // Should preserve subsecond precision - let d = Interpreter::parse_timeout_duration("0.001").unwrap(); + let d = parse_duration("0.001").unwrap(); assert_eq!(d, Duration::from_secs_f64(0.001)); - let d = Interpreter::parse_timeout_duration("0.5").unwrap(); + let d = parse_duration("0.5").unwrap(); assert_eq!(d, Duration::from_millis(500)); - let d = Interpreter::parse_timeout_duration("1.5s").unwrap(); + let d = parse_duration("1.5s").unwrap(); assert_eq!(d, Duration::from_millis(1500)); // Zero should work - let d = Interpreter::parse_timeout_duration("0").unwrap(); + let d = parse_duration("0").unwrap(); assert_eq!(d, Duration::ZERO); } diff --git a/specs/005-builtins.md b/specs/005-builtins.md index e731c69e..abc236c4 100644 --- a/specs/005-builtins.md +++ b/specs/005-builtins.md @@ -184,6 +184,12 @@ let bash = Bash::builder() #[async_trait] pub trait Builtin: Send + Sync { async fn execute(&self, ctx: Context<'_>) -> Result; + + /// Return an execution plan for sub-command execution. + /// Default: Ok(None) — normal execute() is used. + async fn execution_plan(&self, ctx: &Context<'_>) -> Result> { + Ok(None) + } } pub struct Context<'a> { @@ -199,6 +205,44 @@ pub struct Context<'a> { } ``` +### Execution Plans (Sub-Command Delegation) + +Builtins cannot access the interpreter directly. When a builtin needs to run +other commands (e.g. `timeout`, `xargs`, `find -exec`), it returns a declarative +`ExecutionPlan` from `execution_plan()`. The interpreter checks this method +before `execute()` — when it returns `Some(plan)`, the interpreter fulfills the +plan instead of using the `execute()` result. + +```rust +pub struct SubCommand { + pub name: String, + pub args: Vec, + pub stdin: Option, +} + +pub enum ExecutionPlan { + /// Run a single command with a timeout. + Timeout { + duration: Duration, + preserve_status: bool, + command: SubCommand, + }, + /// Run a sequence of commands, collecting output. + Batch { + commands: Vec, + }, +} +``` + +**Current users:** +- `timeout` → `ExecutionPlan::Timeout` — wraps a sub-command with a time limit +- `xargs` → `ExecutionPlan::Batch` — builds commands from stdin lines +- `find -exec` → `ExecutionPlan::Batch` — runs commands on matched files + +**Adding new execution plans:** Add a variant to `ExecutionPlan` and handle it +in the interpreter's plan fulfillment code (`interpreter/mod.rs`). Custom +builtins can also override `execution_plan()` to request sub-command execution. + ### Custom Builtins Bashkit supports registering custom builtins via `BashBuilder`: