fix(processor): refine error handling for file/directory path conflicts

Addresses failing tests observed specifically on the Ubuntu CI runner. These
failures were related to filesystem operations where path components might
exist as files instead of the expected directories.

The previous error handling in the processor could mask these specific
file/directory conflict errors, often reporting them as generic I/O or
path canonicalization failures. This inconsistency likely caused test
assertions (e.g., on summary counts or specific error types) to fail on
the Ubuntu environment.

This change improves the robustness and cross-platform consistency of
error handling by:
- Using `fs::metadata` more proactively in `create.rs` and `safety.rs` to
  check the type of existing path components before proceeding.
- Explicitly catching `io::ErrorKind::NotADirectory` during metadata checks,
  directory creation (`create_dir_all`), file writing (`fs::write`), and
  path canonicalization.
- Mapping these specific file/directory conflict errors to the more precise
  `ProcessError::ParentIsNotDirectory` and `ProcessError::TargetIsDirectory`
  variants.
- Ensuring the `summary_updater` correctly increments specific failure
  counters (`failed_parent_isdir`, `failed_isdir_create`) based on these
  refined error types, leading to more accurate summaries.
- Centralizing summary updates for path format errors in `action_handler.rs`.

This resolves the CI failures by providing more accurate error detection
and reporting, leading to consistent behavior and correct summary counts
across different environments.

Author: Romelium

src/processor/action_handler.rs

@@ -27,54 +27,70 @@ pub(crate) fn process_single_action(
     );
 
     // --- Validate Path Format (String-based check FIRST) ---
+    // (Validation logic remains the same)
     if relative_path_str.contains("//") || relative_path_str.contains(r"\\") {
         eprintln!(
             "Error: Invalid path format (consecutive separators) for '{}'. Skipping.",
             relative_path_str
         );
-        summary.failed_unsafe += 1;
+        // Directly update summary for format errors before path conversion
+        summary_updater::update_summary_error(
+            summary,
+            ProcessError::InvalidPathFormat {
+                path: relative_path_str.to_string(),
+            },
+        );
         return;
     }
-    // Also check for trailing separators (unless it's just root)
     if (relative_path_str.ends_with('/') || relative_path_str.ends_with('\\'))
         && relative_path_str.len() > 1
     {
         eprintln!(
             "Error: Invalid path format (trailing separator) for '{}'. Skipping.",
             relative_path_str
         );
-        summary.failed_unsafe += 1;
+        summary_updater::update_summary_error(
+            summary,
+            ProcessError::InvalidPathFormat {
+                path: relative_path_str.to_string(),
+            },
+        );
         return;
     }
-    // Check for completely empty path string
     if relative_path_str.trim().is_empty() {
         eprintln!("Error: Invalid path format (empty path string). Skipping.");
-        summary.failed_unsafe += 1;
+        summary_updater::update_summary_error(
+            summary,
+            ProcessError::InvalidPathFormat {
+                path: relative_path_str.to_string(),
+            },
+        );
         return;
     }
 
     // --- Convert to PathBuf and check components (redundant but safe) ---
     let relative_path = PathBuf::from(relative_path_str);
-    // This check might be redundant now but doesn't hurt
     if relative_path
         .components()
         .any(|comp| comp.as_os_str().is_empty())
     {
         eprintln!("Error: Invalid path format (empty components detected after PathBuf conversion) for '{}'. Skipping.", relative_path_str);
-        summary.failed_unsafe += 1;
+        summary_updater::update_summary_error(
+            summary,
+            ProcessError::InvalidPathFormat {
+                path: relative_path_str.to_string(),
+            },
+        );
         return;
     }
     let potential_full_path = resolved_base.join(&relative_path);
 
     // --- Safety Check ---
     if let Err(e) = safety::ensure_path_safe(resolved_base, &potential_full_path) {
         eprintln!("Error processing action for '{}': {}", relative_path_str, e);
-        match e {
-            ProcessError::PathNotSafe { .. } => summary.failed_unsafe += 1,
-            ProcessError::PathResolution { .. } | ProcessError::Io { .. } => summary.failed_io += 1,
-            _ => summary.error_other += 1, // Should not happen from safety check
-        }
-        return;
+        // *** Call the summary updater function here ***
+        summary_updater::update_summary_error(summary, e);
+        return; // Exit before dispatching
     }
 
     // --- Dispatch to Action Handler ---
@@ -93,7 +109,10 @@ pub(crate) fn process_single_action(
 
     // --- Handle Errors from Action Handlers ---
     if let Err(e) = result {
+        // Debug log can be removed now if desired
+        // eprintln!("[DEBUG] Action handler returned error variant: {:?}", e);
         eprintln!("Error processing action for '{}': {}", relative_path_str, e);
+        // *** Call the summary updater function here too ***
         summary_updater::update_summary_error(summary, e);
     }
 }

src/processor/create.rs

@@ -3,6 +3,7 @@
 use crate::core_types::{Action, CreateStatus};
 use crate::errors::ProcessError;
 use std::fs;
+use std::io; // Import io for ErrorKind
 use std::path::Path;
 
 /// Creates or overwrites a file with the provided content.
@@ -19,6 +20,7 @@ pub(crate) fn process_create(
         .ok_or_else(|| ProcessError::Internal("Missing content for create action".to_string()))?;
 
     // Ensure parent directory exists and is a directory
+    // This might return ParentIsNotDirectory if parent exists as file or if creation fails
     ensure_parent_directory(resolved_full_path, resolved_base)?;
 
     let mut status = CreateStatus::Created; // Default optimistic status
@@ -53,8 +55,26 @@ pub(crate) fn process_create(
     }
 
     // Write the file content (as bytes to preserve line endings)
-    fs::write(resolved_full_path, content.as_bytes())
-        .map_err(|e| ProcessError::Io { source: e })?;
+    fs::write(resolved_full_path, content.as_bytes()).map_err(|e| {
+        // Check if the write failed because the parent path component is a file
+        if e.kind() == io::ErrorKind::NotADirectory {
+            // Map this specific IO error to our more descriptive error
+            let parent_path = resolved_full_path
+                .parent()
+                .unwrap_or(resolved_full_path)
+                .to_path_buf();
+            // *** DEBUG LOG ***
+            // eprintln!("[DEBUG] fs::write failed with NotADirectory, mapping to ParentIsNotDirectory for path: {}", resolved_full_path.display());
+            ProcessError::ParentIsNotDirectory {
+                path: resolved_full_path.to_path_buf(),
+                parent_path, // Report the parent path
+            }
+        } else {
+            // *** DEBUG LOG ***
+            // eprintln!("[DEBUG] fs::write failed with other IO error: {:?}, mapping to Io for path: {}", e.kind(), resolved_full_path.display());
+            ProcessError::Io { source: e }
+        }
+    })?;
 
     Ok(status)
 }
@@ -63,21 +83,59 @@ pub(crate) fn process_create(
 /// Also checks if the parent path itself is unexpectedly a file.
 fn ensure_parent_directory(target_path: &Path, resolved_base: &Path) -> Result<(), ProcessError> {
     if let Some(parent_dir) = target_path.parent() {
-        if parent_dir == resolved_base || parent_dir.exists() {
-            // If parent exists, ensure it's a directory
-            if !parent_dir.is_dir() {
-                return Err(ProcessError::ParentIsNotDirectory {
-                    path: target_path.to_path_buf(),
-                    parent_path: parent_dir.to_path_buf(),
-                });
+        // Avoid checking the base directory itself if it's the parent
+        if parent_dir == resolved_base || parent_dir.as_os_str().is_empty() {
+            return Ok(()); // Base directory is guaranteed to exist and be a dir, or path is in root
+        }
+
+        match fs::metadata(parent_dir) {
+            Ok(metadata) => {
+                // Parent exists, check if it's a directory
+                if !metadata.is_dir() {
+                    // *** DEBUG LOG ***
+                    // eprintln!("[DEBUG] Parent metadata exists but is not dir, returning ParentIsNotDirectory for parent: {}", parent_dir.display());
+                    return Err(ProcessError::ParentIsNotDirectory {
+                        path: target_path.to_path_buf(),
+                        parent_path: parent_dir.to_path_buf(),
+                    });
+                }
+                // Parent exists and is a directory, all good.
+            }
+            Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
+                // Parent does not exist, try to create it
+                let relative_parent_dir =
+                    parent_dir.strip_prefix(resolved_base).unwrap_or(parent_dir);
+                println!("  Creating directory: {}", relative_parent_dir.display());
+
+                if let Err(create_err) = fs::create_dir_all(parent_dir) {
+                    // Check if the error is specifically "Not a directory"
+                    // This often indicates an intermediate path component was a file during creation attempt.
+                    if create_err.kind() == io::ErrorKind::NotADirectory {
+                        // *** DEBUG LOG ***
+                        // eprintln!("[DEBUG] create_dir_all failed with NotADirectory, returning ParentIsNotDirectory for parent: {}", parent_dir.display());
+                        // Map this specific IO error to our more descriptive error
+                        return Err(ProcessError::ParentIsNotDirectory {
+                            path: target_path.to_path_buf(),
+                            // Report the parent directory we *failed* to create
+                            parent_path: parent_dir.to_path_buf(),
+                        });
+                    } else {
+                        // *** DEBUG LOG ***
+                        // eprintln!("[DEBUG] create_dir_all failed with other IO error: {:?}, returning Io for parent: {}", create_err.kind(), parent_dir.display());
+                        // Other I/O error during creation
+                        return Err(ProcessError::Io { source: create_err });
+                    }
+                }
+                // Creation successful
+            }
+            Err(e) => {
+                // *** DEBUG LOG ***
+                // eprintln!("[DEBUG] fs::metadata failed with other IO error: {:?}, returning Io for parent: {}", e.kind(), parent_dir.display());
+                // Other error getting metadata (permissions?)
+                return Err(ProcessError::Io { source: e });
             }
-        } else {
-            // Parent does not exist, create it
-            let relative_parent_dir = parent_dir.strip_prefix(resolved_base).unwrap_or(parent_dir);
-            println!("  Creating directory: {}", relative_parent_dir.display());
-            fs::create_dir_all(parent_dir).map_err(|e| ProcessError::Io { source: e })?;
         }
     }
-    // If no parent (e.g., root directory), assume it's okay.
+    // If no parent (e.g., file directly in base), assume it's okay.
     Ok(())
 }

src/processor/safety.rs

@@ -1,41 +1,73 @@
 //! Path safety validation logic.
 
 use crate::errors::ProcessError;
-use std::io::ErrorKind;
+use std::fs; // Use fs::metadata
+use std::io::ErrorKind; // Import io and ErrorKind
 use std::path::Path;
 
 /// Checks if the target path is safely within the base directory.
-/// Canonicalizes both paths for reliable comparison.
+/// Canonicalizes paths for reliable comparison.
 pub(crate) fn ensure_path_safe(base_dir: &Path, target_path: &Path) -> Result<(), ProcessError> {
     // Canonicalize base directory (must succeed as it's resolved in process_actions)
-    let canonical_base = base_dir
-        .canonicalize()
-        .map_err(|e| ProcessError::Io { source: e })?;
+    let canonical_base = match base_dir.canonicalize() {
+        Ok(path) => path,
+        // Handle potential NotADirectory error if base_dir itself is somehow a file (should be caught earlier, but belt-and-suspenders)
+        Err(e) if e.kind() == ErrorKind::NotADirectory => {
+            return Err(ProcessError::ParentIsNotDirectory {
+                // Technically the base itself isn't a dir
+                path: base_dir.to_path_buf(),
+                parent_path: base_dir.parent().unwrap_or(base_dir).to_path_buf(),
+            });
+        }
+        Err(e) => return Err(ProcessError::Io { source: e }),
+    };
 
-    // Attempt to canonicalize the target path.
-    match target_path.canonicalize() {
-        Ok(canonical_target) => {
-            // If target exists and canonicalizes, check if it starts with the base
-            if canonical_target.starts_with(&canonical_base) {
-                Ok(()) // Path is safe
-            } else {
-                Err(ProcessError::PathNotSafe {
-                    resolved_path: canonical_target,
-                    base_path: canonical_base,
-                })
+    // Check if the target path *exists* first using metadata.
+    match fs::metadata(target_path) {
+        Ok(_) => {
+            // Target exists. Canonicalize it for the safety check.
+            match target_path.canonicalize() {
+                Ok(canonical_target) => {
+                    if canonical_target.starts_with(&canonical_base) {
+                        Ok(()) // Path exists and is safe
+                    } else {
+                        Err(ProcessError::PathNotSafe {
+                            resolved_path: canonical_target,
+                            base_path: canonical_base,
+                        })
+                    }
+                }
+                // *** CATCH NotADirectory during canonicalization of EXISTING target ***
+                Err(e) if e.kind() == ErrorKind::NotADirectory => {
+                    Err(ProcessError::ParentIsNotDirectory {
+                        path: target_path.to_path_buf(),
+                        parent_path: target_path.parent().unwrap_or(target_path).to_path_buf(),
+                    })
+                }
+                Err(e) => {
+                    // Error canonicalizing an *existing* path (permissions?)
+                    Err(ProcessError::PathResolution {
+                        path: target_path.to_path_buf(),
+                        details: format!("Failed to canonicalize existing target path: {}", e),
+                    })
+                }
             }
         }
         Err(ref e) if e.kind() == ErrorKind::NotFound => {
             // Target doesn't exist: Check safety based on its intended parent.
             check_nonexistent_target_safety(target_path, &canonical_base)
         }
-        Err(e) => {
-            // Other error during canonicalization (e.g., permission denied)
-            Err(ProcessError::PathResolution {
+        // *** CATCH NotADirectory during metadata check (parent is file) ***
+        Err(ref e) if e.kind() == ErrorKind::NotADirectory => {
+            Err(ProcessError::ParentIsNotDirectory {
                 path: target_path.to_path_buf(),
-                details: format!("Failed to canonicalize target path: {}", e),
+                parent_path: target_path.parent().unwrap_or(target_path).to_path_buf(),
             })
         }
+        Err(e) => {
+            // Other error getting metadata (permissions?)
+            Err(ProcessError::Io { source: e }) // Map other metadata errors to IO
+        }
     }
 }
 
@@ -60,6 +92,13 @@ fn check_nonexistent_target_safety(
                     })
                 }
             }
+            // *** CATCH NotADirectory during PARENT canonicalization ***
+            Err(ref parent_err) if parent_err.kind() == ErrorKind::NotADirectory => {
+                Err(ProcessError::ParentIsNotDirectory {
+                    path: target_path.to_path_buf(), // The target we were trying to create
+                    parent_path: parent.to_path_buf(), // The parent that is not a directory
+                })
+            }
             Err(ref parent_err) if parent_err.kind() == ErrorKind::NotFound => {
                 // Parent directory itself doesn't exist. This implies it needs to be
                 // created. Assume `create_dir_all` will handle safety within the base.