Process comments

codex-rs/core/src/codex.rs

@@ -286,7 +286,6 @@ pub(crate) struct Session {
     codex_linux_sandbox_exe: Option<PathBuf>,
     user_shell: shell::Shell,
     show_raw_agent_reasoning: bool,
-    shell_snapshot_path: Option<PathBuf>,
 }
 
 /// The context needed for a single turn of the conversation.
@@ -465,11 +464,6 @@ impl Session {
             cwd,
             disable_response_storage,
         };
-        // TODO(jif) add support for other shells.
-        let shell_snapshot_path = match &default_shell {
-            shell::Shell::Zsh(zsh) => zsh.snapshot_path.clone(),
-            _ => None,
-        };
 
         let sess = Arc::new(Session {
             session_id,
@@ -482,7 +476,6 @@ impl Session {
             codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
             user_shell: default_shell,
             show_raw_agent_reasoning: config.show_raw_agent_reasoning,
-            shell_snapshot_path,
         });
 
         // Dispatch the SessionConfiguredEvent first and then report any errors.
@@ -955,9 +948,6 @@ impl Session {
 impl Drop for Session {
     fn drop(&mut self) {
         self.interrupt_task();
-        if let Some(path) = &self.shell_snapshot_path {
-            shell::delete_shell_snapshot(path);
-        }
     }
 }
 
@@ -2312,7 +2302,7 @@ fn should_translate_shell_command(
         || shell_policy.use_profile
         || matches!(
             shell,
-            crate::shell::Shell::Zsh(zsh) if zsh.snapshot_path.is_some()
+            crate::shell::Shell::Zsh(zsh) if zsh.shell_snapshot.is_some()
         )
 }
 
@@ -2936,6 +2926,7 @@ mod tests {
     use mcp_types::TextContent;
     use pretty_assertions::assert_eq;
     use serde_json::json;
+    use shell::ShellSnapshot;
     use std::collections::HashMap;
     use std::path::PathBuf;
     use std::time::Duration as StdDuration;
@@ -2959,11 +2950,11 @@ mod tests {
         }
     }
 
-    fn zsh_shell(snapshot_path: Option<PathBuf>) -> shell::Shell {
+    fn zsh_shell(shell_snapshot: Option<ShellSnapshot>) -> shell::Shell {
         shell::Shell::Zsh(shell::ZshShell {
             shell_path: "/bin/zsh".to_string(),
             zshrc_path: "/Users/example/.zshrc".to_string(),
-            snapshot_path,
+            shell_snapshot,
         })
     }
 
@@ -2977,7 +2968,7 @@ mod tests {
     #[test]
     fn translates_commands_for_zsh_with_snapshot() {
         let policy = shell_policy_with_profile(false);
-        let shell = zsh_shell(Some(PathBuf::from("/tmp/snapshot")));
+        let shell = zsh_shell(Some(ShellSnapshot::new(PathBuf::from("/tmp/snapshot"))));
         assert!(should_translate_shell_command(&shell, &policy));
     }
 

codex-rs/core/src/shell.rs

@@ -10,7 +10,24 @@ use uuid::Uuid;
 pub struct ZshShell {
     pub(crate) shell_path: String,
     pub(crate) zshrc_path: String,
-    pub(crate) snapshot_path: Option<PathBuf>,
+    pub(crate) shell_snapshot: Option<ShellSnapshot>,
+}
+
+#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
+pub struct ShellSnapshot {
+    pub(crate) path: PathBuf,
+}
+
+impl ShellSnapshot {
+    pub fn new(path: PathBuf) -> Self {
+        Self { path }
+    }
+}
+
+impl Drop for ShellSnapshot {
+    fn drop(&mut self) {
+        delete_shell_snapshot(&self.path);
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
@@ -39,10 +56,10 @@ impl Shell {
 
                 let joined = joined?;
 
-                if let Some(snapshot_path) = &zsh.snapshot_path
-                    && snapshot_path.exists()
+                if let Some(shell_snapshot) = &zsh.shell_snapshot
+                    && shell_snapshot.path.exists()
                 {
-                    let snapshot_path_string = snapshot_path.to_string_lossy();
+                    let snapshot_path_string = shell_snapshot.path.to_string_lossy();
                     trace!(
                         snapshot_path = %snapshot_path_string,
                         "using cached zsh snapshot"
@@ -163,7 +180,7 @@ pub async fn default_user_shell(session_id: Uuid) -> Shell {
                     return Shell::Zsh(ZshShell {
                         shell_path: shell_path.to_string(),
                         zshrc_path: home.join(".zshrc").to_string_lossy().to_string(),
-                        snapshot_path,
+                        shell_snapshot: snapshot_path.map(ShellSnapshot::new),
                     });
                 }
             }
@@ -229,7 +246,9 @@ fn zsh_profile_paths(home: &Path) -> Vec<PathBuf> {
 
 #[cfg(target_os = "macos")]
 async fn ensure_zsh_snapshot(shell_path: &str, home: &Path, session_id: Uuid) -> Option<PathBuf> {
-    let snapshot_path = home.join(format!(".codex_shell_snapshot_{session_id}.zsh"));
+    let snapshot_path = home
+        .join(".codex")
+        .join(format!("codex_shell_snapshot_{session_id}.zsh"));
 
     // Check if an update in the profile requires to re-generate the snapshot.
     let snapshot_is_stale = match tokio::fs::metadata(&snapshot_path).await {
@@ -311,6 +330,10 @@ async fn regenerate_zsh_snapshot(
     contents.push_str(&String::from_utf8_lossy(&output.stdout));
     contents.push('\n');
 
+    if let Some(parent) = snapshot_path.parent() {
+        tokio::fs::create_dir_all(parent).await?;
+    }
+
     let tmp_path = snapshot_path.with_extension("tmp");
     tokio::fs::write(&tmp_path, contents).await?;
 
@@ -371,7 +394,7 @@ mod tests {
         let shell = Shell::Zsh(ZshShell {
             shell_path: "/bin/zsh".to_string(),
             zshrc_path: "/does/not/exist/.zshrc".to_string(),
-            snapshot_path: None,
+            shell_snapshot: None,
         });
         let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]);
         assert_eq!(actual_cmd, None);
@@ -430,7 +453,7 @@ mod tests {
                 std::fs::write(&path, "# test zshrc").unwrap();
                 path.to_string_lossy().to_string()
             },
-            snapshot_path: Some(snapshot_path.clone()),
+            shell_snapshot: Some(ShellSnapshot::new(snapshot_path.clone())),
         });
 
         let invocation = shell.format_default_shell_invocation(vec!["echo".to_string()]);
@@ -501,7 +524,7 @@ mod tests {
             let shell = Shell::Zsh(ZshShell {
                 shell_path: shell_path.to_string(),
                 zshrc_path: zshrc_path.to_str().unwrap().to_string(),
-                snapshot_path: None,
+                shell_snapshot: None,
             });
 
             let actual_cmd = shell