Merge PR #283

Author: factorydroid

Cargo.lock

@@ -15,6 +15,7 @@ workspace = true
 [dependencies]
 cortex-protocol = { workspace = true }
 serde = { workspace = true }
+serde_json = { workspace = true }
 toml = { workspace = true }
 once_cell = { workspace = true }
 clap = { workspace = true, optional = true }

cortex-common/Cargo.toml

@@ -0,0 +1,301 @@
+//! Working directory change guard.
+//!
+//! Provides RAII-style guard for changing working directory that ensures
+//! the original directory is restored on exit or error.
+//!
+//! # Issue Addressed
+//! - #2796: Working directory not restored after error when using --cwd option
+
+use std::env;
+use std::io;
+use std::path::{Path, PathBuf};
+
+/// A guard that restores the working directory when dropped.
+///
+/// This ensures that if an operation changes the working directory and then
+/// fails, the original directory is restored, preventing user confusion.
+///
+/// # Examples
+/// ```ignore
+/// use cortex_common::cwd_guard::CwdGuard;
+///
+/// fn process_in_directory(dir: &Path) -> Result<(), Error> {
+///     let _guard = CwdGuard::new(dir)?;
+///     // Working directory is now `dir`
+///     
+///     do_something_that_might_fail()?;
+///     
+///     // Guard is dropped here, restoring original directory
+///     // even if do_something_that_might_fail() returned an error
+///     Ok(())
+/// }
+/// ```
+#[derive(Debug)]
+pub struct CwdGuard {
+    /// The original working directory to restore
+    original: PathBuf,
+    /// Whether restoration has been disabled
+    disabled: bool,
+}
+
+impl CwdGuard {
+    /// Create a new guard, changing to the specified directory.
+    ///
+    /// # Arguments
+    /// * `new_dir` - The directory to change to
+    ///
+    /// # Returns
+    /// A `Result` containing the guard, or an IO error if the directory
+    /// change failed.
+    ///
+    /// # Examples
+    /// ```ignore
+    /// let guard = CwdGuard::new("/tmp/workspace")?;
+    /// // Now in /tmp/workspace
+    /// // Original directory will be restored when guard is dropped
+    /// ```
+    pub fn new(new_dir: impl AsRef<Path>) -> io::Result<Self> {
+        let original = env::current_dir()?;
+        env::set_current_dir(new_dir.as_ref())?;
+
+        Ok(Self {
+            original,
+            disabled: false,
+        })
+    }
+
+    /// Create a guard that saves the current directory without changing it.
+    ///
+    /// This is useful when you might change directories later but want to
+    /// ensure restoration regardless.
+    ///
+    /// # Returns
+    /// A `Result` containing the guard.
+    pub fn save_current() -> io::Result<Self> {
+        let original = env::current_dir()?;
+
+        Ok(Self {
+            original,
+            disabled: false,
+        })
+    }
+
+    /// Get the original directory that will be restored.
+    pub fn original(&self) -> &Path {
+        &self.original
+    }
+
+    /// Get the current working directory.
+    pub fn current() -> io::Result<PathBuf> {
+        env::current_dir()
+    }
+
+    /// Disable restoration when the guard is dropped.
+    ///
+    /// Use this if you want to keep the new working directory after
+    /// successful completion.
+    pub fn disable_restore(&mut self) {
+        self.disabled = true;
+    }
+
+    /// Enable restoration (undo `disable_restore`).
+    pub fn enable_restore(&mut self) {
+        self.disabled = false;
+    }
+
+    /// Manually restore the original directory early.
+    ///
+    /// This is called automatically on drop, but can be called manually
+    /// if you need to restore earlier.
+    ///
+    /// # Returns
+    /// An `io::Result` indicating success or failure of the directory change.
+    pub fn restore_now(&mut self) -> io::Result<()> {
+        if !self.disabled {
+            env::set_current_dir(&self.original)?;
+            self.disabled = true; // Prevent double-restore on drop
+        }
+        Ok(())
+    }
+
+    /// Change to a new directory while keeping the same restoration point.
+    ///
+    /// # Arguments
+    /// * `new_dir` - The new directory to change to
+    ///
+    /// # Returns
+    /// An `io::Result` indicating success or failure.
+    pub fn change_to(&self, new_dir: impl AsRef<Path>) -> io::Result<()> {
+        env::set_current_dir(new_dir)
+    }
+}
+
+impl Drop for CwdGuard {
+    fn drop(&mut self) {
+        if !self.disabled {
+            // Ignore errors on drop - we can't do much about them
+            // and panicking in drop is bad
+            let _ = env::set_current_dir(&self.original);
+        }
+    }
+}
+
+/// Execute a closure in a specific directory, restoring on completion.
+///
+/// This is a convenience function for simple use cases where you don't
+/// need access to the guard.
+///
+/// # Arguments
+/// * `dir` - The directory to execute in
+/// * `f` - The closure to execute
+///
+/// # Returns
+/// The result of the closure, or an IO error if directory change failed.
+///
+/// # Examples
+/// ```ignore
+/// use cortex_common::cwd_guard::in_directory;
+///
+/// let result = in_directory("/tmp/workspace", || {
+///     // Do work in /tmp/workspace
+///     Ok(compute_something())
+/// })?;
+/// // Back in original directory
+/// ```
+pub fn in_directory<T, F>(dir: impl AsRef<Path>, f: F) -> io::Result<T>
+where
+    F: FnOnce() -> T,
+{
+    let _guard = CwdGuard::new(dir)?;
+    Ok(f())
+}
+
+/// Execute a fallible closure in a specific directory, restoring on completion.
+///
+/// # Arguments
+/// * `dir` - The directory to execute in
+/// * `f` - The closure to execute
+///
+/// # Returns
+/// The result of the closure, or an error.
+pub fn in_directory_result<T, E, F>(dir: impl AsRef<Path>, f: F) -> Result<T, E>
+where
+    E: From<io::Error>,
+    F: FnOnce() -> Result<T, E>,
+{
+    let _guard = CwdGuard::new(dir)?;
+    f()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tempfile::TempDir;
+
+    #[test]
+    fn test_cwd_guard_restores() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+
+        {
+            let _guard = CwdGuard::new(temp_dir.path()).unwrap();
+            assert_eq!(
+                env::current_dir().unwrap(),
+                temp_dir.path().canonicalize().unwrap()
+            );
+        }
+
+        // Should be back to original after guard dropped
+        assert_eq!(env::current_dir().unwrap(), original);
+    }
+
+    #[test]
+    fn test_cwd_guard_restores_on_panic() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+
+        let result = std::panic::catch_unwind(|| {
+            let _guard = CwdGuard::new(temp_dir.path()).unwrap();
+            panic!("test panic");
+        });
+
+        assert!(result.is_err());
+        // Should still be back to original
+        assert_eq!(env::current_dir().unwrap(), original);
+    }
+
+    #[test]
+    fn test_cwd_guard_disabled() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+        let temp_path = temp_dir.path().canonicalize().unwrap();
+
+        {
+            let mut guard = CwdGuard::new(temp_dir.path()).unwrap();
+            guard.disable_restore();
+        }
+
+        // Should NOT be back to original
+        assert_eq!(env::current_dir().unwrap(), temp_path);
+
+        // Restore manually
+        env::set_current_dir(&original).unwrap();
+    }
+
+    #[test]
+    fn test_cwd_guard_restore_now() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+
+        let mut guard = CwdGuard::new(temp_dir.path()).unwrap();
+
+        // Manually restore
+        guard.restore_now().unwrap();
+        assert_eq!(env::current_dir().unwrap(), original);
+
+        // Guard drop should not panic (it's disabled after restore_now)
+    }
+
+    #[test]
+    fn test_in_directory() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+        let temp_path = temp_dir.path().canonicalize().unwrap();
+
+        let result = in_directory(temp_dir.path(), || env::current_dir().unwrap()).unwrap();
+
+        assert_eq!(result, temp_path);
+        assert_eq!(env::current_dir().unwrap(), original);
+    }
+
+    #[test]
+    fn test_in_directory_result() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+
+        let result: Result<(), io::Error> = in_directory_result(temp_dir.path(), || {
+            Err(io::Error::new(io::ErrorKind::Other, "test error"))
+        });
+
+        assert!(result.is_err());
+        // Should still be back to original
+        assert_eq!(env::current_dir().unwrap(), original);
+    }
+
+    #[test]
+    fn test_save_current() {
+        let original = env::current_dir().unwrap();
+        let temp_dir = TempDir::new().unwrap();
+
+        {
+            let guard = CwdGuard::save_current().unwrap();
+            assert_eq!(guard.original(), original);
+
+            // Manually change directory
+            env::set_current_dir(temp_dir.path()).unwrap();
+        }
+
+        // Should be back to original after guard dropped
+        assert_eq!(env::current_dir().unwrap(), original);
+    }
+}