Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions crates/bashkit/src/builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,17 @@ pub trait Builtin: Send + Sync {
}
}

#[async_trait]
impl Builtin for std::sync::Arc<dyn Builtin> {
async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
(**self).execute(ctx).await
}

fn llm_hint(&self) -> Option<&'static str> {
(**self).llm_hint()
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
51 changes: 44 additions & 7 deletions crates/bashkit/src/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,8 @@ pub struct BashToolBuilder {
limits: Option<ExecutionLimits>,
/// Environment variables to set
env_vars: Vec<(String, String)>,
/// Custom builtins (name, implementation)
builtins: Vec<(String, Box<dyn Builtin>)>,
/// Custom builtins (name, implementation). Arc enables reuse across create_bash calls.
builtins: Vec<(String, Arc<dyn Builtin>)>,
}

impl BashToolBuilder {
Expand Down Expand Up @@ -368,7 +368,7 @@ impl BashToolBuilder {
/// If the builtin implements [`Builtin::llm_hint`], its hint will be
/// included in `help()` and `system_prompt()`.
pub fn builtin(mut self, name: impl Into<String>, builtin: Box<dyn Builtin>) -> Self {
self.builtins.push((name.into(), builtin));
self.builtins.push((name.into(), Arc::from(builtin)));
self
}

Expand Down Expand Up @@ -423,7 +423,7 @@ pub struct BashTool {
hostname: Option<String>,
limits: Option<ExecutionLimits>,
env_vars: Vec<(String, String)>,
builtins: Vec<(String, Box<dyn Builtin>)>,
builtins: Vec<(String, Arc<dyn Builtin>)>,
/// Names of custom builtins (for documentation)
builtin_names: Vec<String>,
/// LLM hints from registered builtins
Expand Down Expand Up @@ -452,9 +452,9 @@ impl BashTool {
for (key, value) in &self.env_vars {
builder = builder.env(key, value);
}
// Move builtins out to avoid borrow issues
for (name, builtin) in std::mem::take(&mut self.builtins) {
builder = builder.builtin(name, builtin);
// Clone Arc builtins so they survive across multiple create_bash calls
for (name, builtin) in &self.builtins {
builder = builder.builtin(name.clone(), Box::new(Arc::clone(builtin)));
}

builder.build()
Expand Down Expand Up @@ -1272,4 +1272,41 @@ mod tests {
assert_eq!(req.commands, "echo hello");
assert_eq!(req.timeout_ms, Some(5000));
}

// Issue #422: create_bash should not empty builtins after first call
#[tokio::test]
async fn test_create_bash_preserves_builtins() {
use crate::builtins::{Builtin, Context};
use crate::ExecResult;
use async_trait::async_trait;

struct TestBuiltin;

#[async_trait]
impl Builtin for TestBuiltin {
async fn execute(&self, _ctx: Context<'_>) -> crate::Result<ExecResult> {
Ok(ExecResult::ok("test_output\n"))
}
}

let mut tool = BashToolBuilder::new()
.builtin("testcmd", Box::new(TestBuiltin))
.build();

// First call
let mut bash1 = tool.create_bash();
let result1 = bash1.exec("testcmd").await.unwrap();
assert!(
result1.stdout.contains("test_output"),
"first call should have custom builtin"
);

// Second call should still have the builtin
let mut bash2 = tool.create_bash();
let result2 = bash2.exec("testcmd").await.unwrap();
assert!(
result2.stdout.contains("test_output"),
"second call should still have custom builtin"
);
}
}
Loading