From 532af01032e4eb133e898c6a96e68662c0fbeb6d Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 2 Mar 2026 00:50:29 +0000
Subject: [PATCH] fix(tool): preserve custom builtins across create_bash calls

std::mem::take emptied builtins on first use. Switch to Arc<dyn Builtin>
so builtins are cloned (via Arc) on each create_bash call. Add Builtin
impl for Arc<dyn Builtin> to enable Box wrapping.

Closes #422

https://claude.ai/code/session_01WZjYqxm5xMPAEe7FSHJkDy
---
 crates/bashkit/src/builtins/mod.rs | 11 +++++++
 crates/bashkit/src/tool.rs         | 51 ++++++++++++++++++++++++++----
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/crates/bashkit/src/builtins/mod.rs b/crates/bashkit/src/builtins/mod.rs
index 8585fa88..c0a2139e 100644
--- a/crates/bashkit/src/builtins/mod.rs
+++ b/crates/bashkit/src/builtins/mod.rs
@@ -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::*;
diff --git a/crates/bashkit/src/tool.rs b/crates/bashkit/src/tool.rs
index ddb3b3be..33c6a410 100644
--- a/crates/bashkit/src/tool.rs
+++ b/crates/bashkit/src/tool.rs
@@ -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 {
@@ -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
     }
 
@@ -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
@@ -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()
@@ -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"
+        );
+    }
 }