Persist terminal tempdirs by thread (#57878)

Summary:
- Store a lazily-created sandboxed terminal temp directory on each agent
thread.
- Persist the tempdir path and remove it when deleting archived threads.
- Update sandbox prompt wording to describe per-thread tempdir reuse.

Tests:
- cargo fmt --package agent --package agent_ui --package zed
- cargo test -p agent db::tests::test_sandboxed_terminal_temp_dir --
--nocapture
- cargo test -p agent
db::tests::test_delete_thread_removes_sandboxed_terminal_temp_dir --
--nocapture
- cargo check -p agent_ui -p zed

Release Notes:

- Improved agent terminal sandboxing to preserve temporary files across
commands in the same thread

---------

Co-authored-by: MartinYe1234 <52641447+MartinYe1234@users.noreply.github.com>
Co-authored-by: Martin Ye <martin@zed.dev>

crates/agent/src/agent.rs

@@ -2616,6 +2616,38 @@ impl ThreadEnvironment for NativeThreadEnvironment {
         sandbox_wrap: Option<acp_thread::SandboxWrap>,
         cx: &mut AsyncApp,
     ) -> Task<Result<Rc<dyn TerminalHandle>>> {
+        // Use a per-thread temp directory for all terminal commands, even when
+        // sandboxing is disabled, so the model can't infer sandbox state from
+        // `$TMPDIR` changing between conversations.
+        let mut extra_env = extra_env;
+        let mut sandbox_wrap = sandbox_wrap;
+        match self
+            .thread
+            .update(cx, |thread, cx| thread.sandboxed_terminal_temp_dir(cx))
+        {
+            Ok(Ok(temp_dir)) => {
+                // Canonicalize so the path matches what the sandbox resolves
+                // symlinks to (e.g. `/var` -> `/private/var` on macOS).
+                // `$TMPDIR` and the writable-scope entry below must agree, and
+                // they must agree with the path the kernel actually checks.
+                let temp_dir = temp_dir.canonicalize().unwrap_or(temp_dir);
+                let temp_dir_string = temp_dir.to_string_lossy().into_owned();
+                extra_env.extend([
+                    acp::EnvVariable::new("TMPDIR", &temp_dir_string),
+                    acp::EnvVariable::new("TMP", &temp_dir_string),
+                    acp::EnvVariable::new("TEMP", &temp_dir_string),
+                ]);
+                // The command's `$TMPDIR` must live inside the sandbox's
+                // writable scope. The per-thread temp directory is owned here
+                // (not in the terminal tool that assembles the rest of the
+                // writable set), so add it whenever the command is sandboxed.
+                if let Some(sandbox_wrap) = &mut sandbox_wrap {
+                    sandbox_wrap.writable_paths.push(temp_dir);
+                }
+            }
+            Ok(Err(error)) => return Task::ready(Err(error)),
+            Err(error) => return Task::ready(Err(error)),
+        };
         let task = self.acp_thread.update(cx, |thread, cx| {
             thread.create_terminal(
                 command,

crates/agent/src/db.rs

@@ -16,7 +16,7 @@ use sqlez::{
     connection::Connection,
     statement::Statement,
 };
-use std::sync::Arc;
+use std::{io::ErrorKind, path::PathBuf, sync::Arc};
 use ui::{App, SharedString};
 use util::path_list::PathList;
 use zed_env_vars::ZED_STATELESS;
@@ -81,6 +81,8 @@ pub struct DbThread {
     pub draft_prompt: Option<Vec<acp::ContentBlock>>,
     #[serde(default)]
     pub ui_scroll_position: Option<SerializedScrollPosition>,
+    #[serde(default)]
+    pub sandboxed_terminal_temp_dir: Option<PathBuf>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
@@ -130,6 +132,7 @@ impl SharedThread {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         }
     }
 
@@ -309,6 +312,7 @@ impl DbThread {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         })
     }
 }
@@ -569,15 +573,7 @@ impl ThreadsDatabase {
 
             let rows = select(id.0)?;
             if let Some((data_type, data)) = rows.into_iter().next() {
-                let json_data = match data_type {
-                    DataType::Zstd => {
-                        let decompressed = zstd::decode_all(&data[..])?;
-                        String::from_utf8(decompressed)?
-                    }
-                    DataType::Json => String::from_utf8(data)?,
-                };
-                let thread = DbThread::from_json(json_data.as_bytes())?;
-                Ok(Some(thread))
+                Ok(Some(Self::deserialize_thread(data_type, data)?))
             } else {
                 Ok(None)
             }
@@ -596,17 +592,71 @@ impl ThreadsDatabase {
             .spawn(async move { Self::save_thread_sync(&connection, id, thread, &folder_paths) })
     }
 
+    fn deserialize_thread(data_type: DataType, data: Vec<u8>) -> Result<DbThread> {
+        let json_data = match data_type {
+            DataType::Zstd => {
+                let decompressed = zstd::decode_all(&data[..])?;
+                String::from_utf8(decompressed)?
+            }
+            DataType::Json => String::from_utf8(data)?,
+        };
+        DbThread::from_json(json_data.as_bytes())
+    }
+
+    fn sandboxed_terminal_temp_dir(data_type: DataType, data: Vec<u8>) -> Option<PathBuf> {
+        match Self::deserialize_thread(data_type, data) {
+            Ok(thread) => thread.sandboxed_terminal_temp_dir,
+            Err(error) => {
+                log::warn!("failed to deserialize thread before deleting it: {error:#}");
+                None
+            }
+        }
+    }
+
+    fn remove_sandboxed_terminal_temp_dir(temp_dir: PathBuf) {
+        match std::fs::remove_dir_all(&temp_dir) {
+            Ok(()) => {}
+            Err(error) if error.kind() == ErrorKind::NotFound => {}
+            Err(error) => {
+                log::warn!(
+                    "failed to remove sandboxed terminal temp directory {}: {error}",
+                    temp_dir.display()
+                );
+            }
+        }
+    }
+
     pub fn delete_thread(&self, id: acp::SessionId) -> Task<Result<()>> {
         let connection = self.connection.clone();
 
         self.executor.spawn(async move {
-            let connection = connection.lock();
+            let sandboxed_terminal_temp_dir = {
+                let connection = connection.lock();
 
-            let mut delete = connection.exec_bound::<Arc<str>>(indoc! {"
-                DELETE FROM threads WHERE id = ?
-            "})?;
+                let mut select =
+                    connection.select_bound::<Arc<str>, (DataType, Vec<u8>)>(indoc! {"
+                    SELECT data_type, data FROM threads WHERE id = ? LIMIT 1
+                "})?;
 
-            delete(id.0)?;
+                let sandboxed_terminal_temp_dir = select(id.0.clone())?
+                    .into_iter()
+                    .next()
+                    .and_then(|(data_type, data)| {
+                        Self::sandboxed_terminal_temp_dir(data_type, data)
+                    });
+
+                let mut delete = connection.exec_bound::<Arc<str>>(indoc! {"
+                    DELETE FROM threads WHERE id = ?
+                "})?;
+
+                delete(id.0)?;
+
+                sandboxed_terminal_temp_dir
+            };
+
+            if let Some(temp_dir) = sandboxed_terminal_temp_dir {
+                Self::remove_sandboxed_terminal_temp_dir(temp_dir);
+            }
 
             Ok(())
         })
@@ -616,13 +666,32 @@ impl ThreadsDatabase {
         let connection = self.connection.clone();
 
         self.executor.spawn(async move {
-            let connection = connection.lock();
+            let sandboxed_terminal_temp_dirs = {
+                let connection = connection.lock();
 
-            let mut delete = connection.exec_bound::<()>(indoc! {"
-                DELETE FROM threads
-            "})?;
+                let mut select = connection.select_bound::<(), (DataType, Vec<u8>)>(indoc! {"
+                    SELECT data_type, data FROM threads
+                "})?;
 
-            delete(())?;
+                let sandboxed_terminal_temp_dirs = select(())?
+                    .into_iter()
+                    .filter_map(|(data_type, data)| {
+                        Self::sandboxed_terminal_temp_dir(data_type, data)
+                    })
+                    .collect::<Vec<_>>();
+
+                let mut delete = connection.exec_bound::<()>(indoc! {"
+                    DELETE FROM threads
+                "})?;
+
+                delete(())?;
+
+                sandboxed_terminal_temp_dirs
+            };
+
+            for temp_dir in sandboxed_terminal_temp_dirs {
+                Self::remove_sandboxed_terminal_temp_dir(temp_dir);
+            }
 
             Ok(())
         })
@@ -694,6 +763,7 @@ mod tests {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         }
     }
 
@@ -797,6 +867,78 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_sandboxed_terminal_temp_dir_defaults_to_none() {
+        let json = r#"{
+            "title": "Old Thread",
+            "messages": [],
+            "updated_at": "2024-01-01T00:00:00Z"
+        }"#;
+
+        let db_thread: DbThread = serde_json::from_str(json).expect("Failed to deserialize");
+
+        assert!(
+            db_thread.sandboxed_terminal_temp_dir.is_none(),
+            "Legacy threads without sandboxed_terminal_temp_dir should default to None"
+        );
+    }
+
+    #[gpui::test]
+    async fn test_sandboxed_terminal_temp_dir_roundtrips_through_save_load(
+        cx: &mut TestAppContext,
+    ) {
+        let database = ThreadsDatabase::new(cx.executor()).unwrap();
+        let thread_id = session_id("sandbox-temp-dir-thread");
+        let temp_dir = tempfile::Builder::new()
+            .prefix("zed-agent-terminal-test-")
+            .tempdir()
+            .unwrap()
+            .keep();
+        let mut thread = make_thread(
+            "Sandbox Temp Dir Thread",
+            Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap(),
+        );
+        thread.sandboxed_terminal_temp_dir = Some(temp_dir.clone());
+
+        database
+            .save_thread(thread_id.clone(), thread, PathList::default())
+            .await
+            .unwrap();
+
+        let loaded = database
+            .load_thread(thread_id)
+            .await
+            .unwrap()
+            .expect("thread should exist");
+        assert_eq!(loaded.sandboxed_terminal_temp_dir, Some(temp_dir.clone()));
+        std::fs::remove_dir_all(temp_dir).unwrap();
+    }
+
+    #[gpui::test]
+    async fn test_delete_thread_removes_sandboxed_terminal_temp_dir(cx: &mut TestAppContext) {
+        let database = ThreadsDatabase::new(cx.executor()).unwrap();
+        let thread_id = session_id("sandbox-temp-dir-delete-thread");
+        let temp_dir = tempfile::Builder::new()
+            .prefix("zed-agent-terminal-test-")
+            .tempdir()
+            .unwrap()
+            .keep();
+        std::fs::write(temp_dir.join("sentinel"), b"content").unwrap();
+        let mut thread = make_thread(
+            "Sandbox Temp Dir Delete Thread",
+            Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap(),
+        );
+        thread.sandboxed_terminal_temp_dir = Some(temp_dir.clone());
+
+        database
+            .save_thread(thread_id.clone(), thread, PathList::default())
+            .await
+            .unwrap();
+        database.delete_thread(thread_id).await.unwrap();
+
+        assert!(!temp_dir.exists());
+    }
+
     #[gpui::test]
     async fn test_subagent_context_roundtrips_through_save_load(cx: &mut TestAppContext) {
         let database = ThreadsDatabase::new(cx.executor()).unwrap();

crates/agent/src/templates.rs

@@ -195,8 +195,6 @@ mod tests {
         assert!(rendered.contains("allow_network: true"));
         assert!(rendered.contains("allow_fs_write: true"));
         assert!(rendered.contains("unsandboxed: true"));
-        // The model is told the section is stable so it doesn't re-check
-        // sandbox state every turn.
         assert!(rendered.contains("remain in effect for the entire duration"));
     }
 

crates/agent/src/templates/system_prompt.hbs

@@ -193,7 +193,7 @@ The current project contains the following root directories:
 The `terminal` tool runs commands inside a sandbox with these permissions:
 
 - Reads: any path on the filesystem is readable.
-- Writes: a per-command temporary directory exposed via `$TMPDIR`, `$TMP`, and `$TEMP` is writable{{#if worktrees}}, along with these project directories:
+- Writes: a per-thread temporary directory exposed via `$TMPDIR`, `$TMP`, and `$TEMP` is writable and persists across `terminal` calls in this conversation{{#if worktrees}}, along with these project directories:
 {{#each worktrees}}
   - `{{abs_path}}`
 {{/each}}
@@ -202,7 +202,7 @@ The `terminal` tool runs commands inside a sandbox with these permissions:
 
 You can request elevated permissions on individual `terminal` calls by setting `allow_network: true`, `allow_fs_write: true`, or `unsandboxed: true`. The user will be prompted to approve before the command runs.
 
-These sandbox settings are guaranteed to remain in effect for the entire duration of this conversation. If they ever change, you'll be told.
+These sandbox settings are guaranteed to remain in effect for the entire duration of this conversation. If they ever change, you will be told.
 
 {{/if}}
 {{#if model_name}}

crates/agent/src/thread.rs

@@ -51,16 +51,16 @@ use serde::{Deserialize, Serialize};
 use settings::{
     LanguageModelSelection, Settings, SettingsStore, ToolPermissionMode, update_settings_file,
 };
+use std::fmt::Write;
 use std::{
     collections::BTreeMap,
     marker::PhantomData,
     ops::RangeInclusive,
-    path::Path,
+    path::{Path, PathBuf},
     rc::Rc,
     sync::Arc,
     time::{Duration, Instant},
 };
-use std::{fmt::Write, path::PathBuf};
 use util::{ResultExt, debug_panic, markdown::MarkdownCodeBlock, paths::PathStyle};
 use uuid::Uuid;
 
@@ -1007,6 +1007,7 @@ pub struct Thread {
     /// Weak references to running subagent threads for cancellation propagation
     running_subagents: Vec<WeakEntity<Thread>>,
     inherits_parent_model_settings: bool,
+    sandboxed_terminal_temp_dir: Option<PathBuf>,
 }
 
 impl Thread {
@@ -1133,6 +1134,7 @@ impl Thread {
             ui_scroll_position: None,
             running_subagents: Vec::new(),
             inherits_parent_model_settings: true,
+            sandboxed_terminal_temp_dir: None,
         }
     }
 
@@ -1176,6 +1178,30 @@ impl Thread {
         &self.id
     }
 
+    pub(crate) fn sandboxed_terminal_temp_dir(
+        &mut self,
+        cx: &mut Context<Self>,
+    ) -> Result<PathBuf> {
+        if let Some(temp_dir) = &self.sandboxed_terminal_temp_dir {
+            std::fs::create_dir_all(temp_dir).with_context(|| {
+                format!(
+                    "failed to recreate sandboxed terminal temp directory {}",
+                    temp_dir.display()
+                )
+            })?;
+            return Ok(temp_dir.clone());
+        }
+
+        let temp_dir = tempfile::Builder::new()
+            .prefix("zed-agent-terminal-")
+            .tempdir()
+            .context("failed to create sandboxed terminal temp directory")?;
+        let temp_dir = temp_dir.keep();
+        self.sandboxed_terminal_temp_dir = Some(temp_dir.clone());
+        cx.notify();
+        Ok(temp_dir)
+    }
+
     /// Returns true if this thread was imported from a shared thread.
     pub fn is_imported(&self) -> bool {
         self.imported
@@ -1451,6 +1477,7 @@ impl Thread {
             }),
             running_subagents: Vec::new(),
             inherits_parent_model_settings: true,
+            sandboxed_terminal_temp_dir: db_thread.sandboxed_terminal_temp_dir,
         }
     }
 
@@ -1481,6 +1508,7 @@ impl Thread {
                     offset_in_item: lo.offset_in_item.as_f32(),
                 }
             }),
+            sandboxed_terminal_temp_dir: self.sandboxed_terminal_temp_dir.clone(),
         };
 
         cx.background_spawn(async move {

crates/agent/src/thread_store.rs

@@ -167,6 +167,7 @@ mod tests {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         }
     }
 

crates/agent/src/tools/terminal_tool.rs

@@ -178,46 +178,31 @@ impl AgentTool for TerminalTool {
                 }
             }
 
-            // Always provision a per-command temporary directory and point
-            // TMPDIR/TMP/TEMP at it. This is independent of sandbox state:
-            // even an unsandboxed command (or a command on a non-macOS
-            // host) gets a clean, isolated scratch directory that's
-            // auto-cleaned when the task ends, rather than polluting the
-            // user's shared `/tmp`. Decoupling it from sandbox state also
-            // means the model can't infer the sandbox state by looking at
-            // `$TMPDIR`.
-            let temp_dir = tempfile::Builder::new()
-                .prefix("zed-agent-terminal-")
-                .tempdir()
-                .map_err(|e| {
-                    format!("failed to create per-command temporary directory: {e}")
-                })?;
-            let temp_dir_path = temp_dir
-                .path()
-                .canonicalize()
-                .unwrap_or_else(|_| temp_dir.path().to_path_buf());
-            let temp_dir_string = temp_dir_path.to_string_lossy().into_owned();
-            let extra_env = vec![
-                acp::EnvVariable::new("TMPDIR", &temp_dir_string),
-                acp::EnvVariable::new("TMP", &temp_dir_string),
-                acp::EnvVariable::new("TEMP", &temp_dir_string),
-            ];
+            // The per-thread scratch directory (and the `$TMPDIR`/`TMP`/
+            // `TEMP` environment variables pointing at it) is provisioned by
+            // the thread environment in `create_terminal`, which also adds it
+            // to the sandbox's writable scope. We must not set `$TMPDIR` here:
+            // the environment overrides it with the per-thread directory, so a
+            // per-command directory set here would never be the `$TMPDIR` the
+            // command actually sees and would be left out of the writable
+            // scope, breaking writes into `$TMPDIR`.
+            let extra_env = Vec::new();
 
-            // Build the writable scope from the project's worktrees plus
-            // this command's temporary directory. Crucially we do *not*
-            // include the resolved `cd` working directory — that's
+            // Build the writable scope from the project's worktrees. The
+            // per-thread temp directory is appended by the thread environment
+            // (which owns it and points `$TMPDIR` at it). Crucially we do
+            // *not* include the resolved `cd` working directory — that's
             // model-controlled, and using it as the writable scope would
             // let the model widen its own write permissions outside the
             // project.
             let sandbox_wrap = if sandboxing && !want_unsandboxed {
-                let mut writable_paths: Vec<PathBuf> = cx.update(|cx| {
+                let writable_paths: Vec<PathBuf> = cx.update(|cx| {
                     self.project
                         .read(cx)
                         .worktrees(cx)
                         .map(|w| w.read(cx).abs_path().to_path_buf())
                         .collect::<Vec<_>>()
                 });
-                writable_paths.push(temp_dir_path.clone());
                 Some(acp_thread::SandboxWrap {
                     writable_paths,
                     allow_network: want_network,
@@ -239,10 +224,6 @@ impl AgentTool for TerminalTool {
                 )
                 .await
                 .map_err(|e| e.to_string())?;
-            // Hold the TempDir until the spawned command has finished so
-            // its scratch contents (and `$TMPDIR`) stay valid for the
-            // command's lifetime.
-            let _temp_dir = temp_dir;
 
             let terminal_id = terminal.id(cx).map_err(|e| e.to_string())?;
             event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![

crates/agent_ui/src/agent_panel.rs

@@ -9748,6 +9748,7 @@ mod tests {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         };
 
         let thread_store = cx.update(|cx| ThreadStore::global(cx));

crates/agent_ui/src/thread_metadata_store.rs

@@ -1831,6 +1831,7 @@ mod tests {
             thinking_effort: None,
             draft_prompt: None,
             ui_scroll_position: None,
+            sandboxed_terminal_temp_dir: None,
         }
     }
 

crates/zed/src/visual_test_runner.rs

@@ -2734,6 +2734,7 @@ fn run_multi_workspace_sidebar_visual_tests(
                             thinking_effort: None,
                             ui_scroll_position: None,
                             draft_prompt: None,
+                            sandboxed_terminal_temp_dir: None,
                         },
                         path_list,
                         cx,