fix(fs): atomic writes, real MultiEdit atomicity, bounded glob results

src/tools/file_edit.rs

@@ -93,7 +93,7 @@ impl Tool for FileEditTool {
                     path.display()
                 )));
             }
-            fs::write(&path, &new_content).await?;
+            super::atomic_write(&path, &new_content).await?;
             return Ok(ToolOutput::success(format!(
                 "Replaced {count} occurrence(s) in {}",
                 path.display()
@@ -109,7 +109,7 @@ impl Tool for FileEditTool {
             ))),
             1 => {
                 let new_content = content.replacen(&input.old_string, &input.new_string, 1);
-                fs::write(&path, &new_content).await?;
+                super::atomic_write(&path, &new_content).await?;
                 Ok(ToolOutput::success(format!(
                     "Edit applied successfully to {}",
                     path.display()

src/tools/file_write.rs

@@ -67,7 +67,7 @@ impl Tool for FileWriteTool {
             })?;
         }
 
-        fs::write(&path, &input.content)
+        super::atomic_write(&path, &input.content)
             .await
             .map_err(|e| anyhow::anyhow!("Failed to write {}: {}", path.display(), e))?;
 

src/tools/glob.rs

@@ -9,6 +9,11 @@ use serde_json::json;
 use std::path::Path;
 use std::time::SystemTime;
 
+/// Cap on returned matches. Generous enough that ordinary searches are never
+/// truncated, low enough that a repo-wide `**/*` cannot produce a multi-megabyte
+/// tool result.
+const MAX_GLOB_RESULTS: usize = 1000;
+
 pub struct GlobTool;
 
 #[derive(Deserialize)]
@@ -100,16 +105,34 @@ impl Tool for GlobTool {
         // Sort by modification time, most recent first
         entries.sort_by_key(|e| std::cmp::Reverse(e.0));
 
+        // Bound the result. A broad pattern over a large tree (`**/*`) otherwise
+        // builds an unbounded Vec and then joins it into one enormous string
+        // that is sent to the model as a tool result — cost and memory scale
+        // with the repository, and nothing downstream caps it. Sorting first
+        // means the cap keeps the most recently modified matches, which is what
+        // the ordering exists to surface.
+        let total = entries.len();
+        let truncated = total > MAX_GLOB_RESULTS;
+        entries.truncate(MAX_GLOB_RESULTS);
+
         if entries.is_empty() {
             return Ok(ToolOutput::success("No files matched the pattern."));
         }
 
-        let output = entries
+        let mut output = entries
             .into_iter()
             .map(|(_, path)| path)
             .collect::<Vec<_>>()
             .join("\n");
 
+        if truncated {
+            output.push_str(&format!(
+                "\n\n... {} of {total} matches shown (most recently modified first). \
+                 Narrow the pattern or search a subdirectory to see the rest.",
+                MAX_GLOB_RESULTS
+            ));
+        }
+
         Ok(ToolOutput::success(output))
     }
 }

src/tools/mod.rs

@@ -268,6 +268,68 @@ fn is_dotenv(file_name: &str) -> bool {
 /// Returns Some(error ToolOutput) if the path should be blocked for `op`.
 /// Read mode blocks private keys only. Write mode additionally blocks
 /// dotenv files, credential stores, and secrets directories.
+/// Write a file atomically: temp file in the same directory, fsync, rename.
+///
+/// A direct `fs::write` truncates the target first, so a crash, a full disk, or
+/// a kill between truncate and write leaves the user's file empty or partial —
+/// silently, and with the original gone. `session/` was given this treatment in
+/// an earlier audit; the file tools were not.
+///
+/// The temp file is created in the *same directory* so the rename is a
+/// same-filesystem operation and therefore atomic; `/tmp` may be a different
+/// mount, where rename degrades to copy-then-delete and loses the guarantee.
+///
+/// **Permissions are carried over from the original.** Renaming replaces the
+/// inode, so without this an edit to a `0600` file would silently republish it
+/// at the default `0644` — turning a routine edit into a disclosure.
+pub async fn atomic_write(path: &std::path::Path, content: &str) -> std::io::Result<()> {
+    use std::sync::atomic::{AtomicU64, Ordering};
+    static SEQ: AtomicU64 = AtomicU64::new(0);
+
+    let parent = path.parent().unwrap_or_else(|| std::path::Path::new("."));
+    let stem = path.file_name().and_then(|n| n.to_str()).unwrap_or("file");
+    let tmp = parent.join(format!(
+        ".{stem}.rustyclaw-{}-{}.tmp",
+        std::process::id(),
+        SEQ.fetch_add(1, Ordering::Relaxed)
+    ));
+
+    // Mode of the file we are replacing, if it exists.
+    #[cfg(unix)]
+    let mode = {
+        use std::os::unix::fs::PermissionsExt;
+        tokio::fs::metadata(path)
+            .await
+            .ok()
+            .map(|m| m.permissions().mode())
+    };
+
+    let write_result = async {
+        let mut f = tokio::fs::File::create(&tmp).await?;
+        tokio::io::AsyncWriteExt::write_all(&mut f, content.as_bytes()).await?;
+        // Durability: without this the rename can land before the data does, so
+        // a crash yields a present-but-empty file — the exact outcome this is
+        // meant to prevent.
+        f.sync_all().await?;
+        drop(f);
+
+        #[cfg(unix)]
+        if let Some(mode) = mode {
+            use std::os::unix::fs::PermissionsExt;
+            tokio::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(mode)).await?;
+        }
+
+        tokio::fs::rename(&tmp, path).await
+    }
+    .await;
+
+    if write_result.is_err() {
+        // Never leave a stray temp file behind on failure.
+        let _ = tokio::fs::remove_file(&tmp).await;
+    }
+    write_result
+}
+
 /// Deny-listed read paths expressed as ripgrep exclusion globs.
 ///
 /// The Grep tool has two backends — a `ripgrep` subprocess and a pure-Rust

src/tools/multi_edit.rs

@@ -91,6 +91,13 @@ impl Tool for MultiEditTool {
         let mut results: Vec<String> = Vec::new();
         let mut had_error = false;
 
+        // path -> fully-edited content, written only if every edit to that path
+        // succeeded.
+        let mut staged: std::collections::BTreeMap<std::path::PathBuf, String> =
+            std::collections::BTreeMap::new();
+        let mut failed_files: std::collections::BTreeSet<std::path::PathBuf> =
+            std::collections::BTreeSet::new();
+
         for (i, edit) in input.edits.iter().enumerate() {
             let path = match resolve_path(&edit.file_path, &ctx.cwd) {
                 Ok(p) => p,
@@ -115,6 +122,7 @@ impl Tool for MultiEditTool {
                     .join(" ");
                 results.push(format!("{label} ✗ {msg}"));
                 had_error = true;
+                failed_files.insert(path.clone());
                 continue;
             }
             if let Some(err) = super::check_sensitive_path_resolved(&path, super::SensitiveOp::Write) {
@@ -138,61 +146,90 @@ impl Tool for MultiEditTool {
             if !path.exists() {
                 results.push(format!("{} ✗ File not found", label));
                 had_error = true;
+                failed_files.insert(path.clone());
                 continue;
             }
 
-            let content = match fs::read_to_string(&path).await {
-                Ok(c) => c,
-                Err(e) => {
-                    results.push(format!("{} ✗ Read error: {}", label, e));
-                    had_error = true;
-                    continue;
-                }
+            // Later edits must see earlier ones. Previously each edit re-read
+            // the file, which worked only because each write landed
+            // immediately — the very thing that made this non-atomic.
+            let content = match staged.get(&path) {
+                Some(c) => c.clone(),
+                None => match fs::read_to_string(&path).await {
+                    Ok(c) => c,
+                    Err(e) => {
+                        results.push(format!("{} ✗ Read error: {}", label, e));
+                        had_error = true;
+                        failed_files.insert(path.clone());
+                        continue;
+                    }
+                },
             };
 
             if edit.replace_all {
                 let count = content.matches(&edit.old_string as &str).count();
                 if count == 0 {
                     results.push(format!("{} ✗ old_string not found", label));
                     had_error = true;
+                    failed_files.insert(path.clone());
                     continue;
                 }
                 let new_content = content.replace(&edit.old_string, &edit.new_string);
-                match fs::write(&path, &new_content).await {
-                    Ok(_) => results.push(format!("{} ✓ Replaced {} occurrence(s)", label, count)),
-                    Err(e) => {
-                        results.push(format!("{} ✗ Write error: {}", label, e));
-                        had_error = true;
-                    }
-                }
+                staged.insert(path.clone(), new_content);
+                results.push(format!("{} ✓ Replaced {} occurrence(s)", label, count));
             } else {
                 let count = content.matches(&edit.old_string as &str).count();
                 match count {
                     0 => {
                         results.push(format!("{} ✗ old_string not found", label));
                         had_error = true;
+                        failed_files.insert(path.clone());
                     }
                     1 => {
                         let new_content = content.replacen(&edit.old_string, &edit.new_string, 1);
-                        match fs::write(&path, &new_content).await {
-                            Ok(_) => results.push(format!("{} ✓ Edit applied", label)),
-                            Err(e) => {
-                                results.push(format!("{} ✗ Write error: {}", label, e));
-                                had_error = true;
-                            }
-                        }
+                        staged.insert(path.clone(), new_content);
+                        results.push(format!("{} ✓ Edit applied", label));
                     }
                     n => {
                         results.push(format!(
                             "{} ✗ old_string found {} times — add more context or use replace_all:true",
                             label, n
                         ));
                         had_error = true;
+                        failed_files.insert(path.clone());
                     }
                 }
             }
         }
 
+        // Commit phase — per-file all-or-nothing.
+        //
+        // Writing inside the loop meant a batch where edit 3 of 5 failed left
+        // the file with edits 1, 2, 4 and 5 applied: a half-finished refactor,
+        // on disk, reported only as one "✗" among several "✓". The doc comment
+        // promised these were applied atomically; now they are.
+        //
+        // Granularity is per file, not per batch: a failure in file A should not
+        // discard good edits to file B.
+        let mut committed = 0usize;
+        for (path, content) in &staged {
+            if failed_files.contains(path) {
+                results.push(format!(
+                    "  ↩ {} — no changes written; another edit to this file failed",
+                    path.display()
+                ));
+                continue;
+            }
+            match super::atomic_write(path, content).await {
+                Ok(_) => committed += 1,
+                Err(e) => {
+                    results.push(format!("  ✗ {} write error: {e}", path.display()));
+                    had_error = true;
+                }
+            }
+        }
+        let _ = committed;
+
         let summary = results.join("\n");
         if had_error {
             Ok(ToolOutput::error(summary))