diff --git a/builtin/runtime/autosave.lua b/builtin/runtime/autosave.lua index 7f005b7..d66e15d 100644 --- a/builtin/runtime/autosave.lua +++ b/builtin/runtime/autosave.lua @@ -48,10 +48,19 @@ function pmacs.autosave.interval_ms(ms) return interval end --- sweep() --- force a pass now. Returns how many buffers were written. +-- sweep() --- force a pass now. Returns (written, blocked). `blocked` +-- counts buffers whose sweep was refused because an unclaimed crash +-- recovery sits at their key: overwriting it would destroy exactly what +-- autosave protects. Recovering or discarding that copy resumes autosave. function pmacs.autosave.sweep() - if not enabled then return 0 end - return pmacs.autosave._sweep() + if not enabled then return 0, 0 end + local written, blocked = pmacs.autosave._sweep() + if blocked and blocked > 0 then + pmacs.editor.set_status( + "autosave paused for " .. blocked .. " file(s) with unclaimed recovery" + .. " --- M-x recover-file or M-x discard-recovery") + end + return written, blocked end local function basename(path) @@ -90,7 +99,7 @@ pmacs.hook.add("process.after-tick", function() end if now - last_sweep_ms >= interval then last_sweep_ms = now - pcall(pmacs.autosave._sweep) + pcall(pmacs.autosave.sweep) end end) @@ -98,21 +107,24 @@ end) -- inside the hook -- a desktop restore fires this once per leaf). pmacs.hook.add("buffer.after-load", function() needs_report = true - -- A clean save or a kill retires the recovery copy. There is no global - -- kill hook, so register per buffer, capturing the path now (the buffer - -- may be gone by the time the callback runs). - local path = pmacs.editor.file_path() - if not path then return end + -- A kill retires the recovery copy. There is no global kill hook, so + -- register per buffer. `_discard_buffer` is keyed by BufferId, not by a + -- path captured here: after a rename the buffer's recovery lives under + -- a different key than the path it loaded with. Buffers that fire no + -- after-load (argv `[new file]`) are covered by the sweep-time GC. local buf = pmacs.window.buffer() if not buf then return end - pcall(pmacs.buffer.on_removed, buf, function() - pcall(pmacs.autosave._discard, path) + pcall(pmacs.buffer.on_removed, buf, function(dead) + pcall(pmacs.autosave._discard_buffer, dead or buf) end) end) +-- A clean save retires the recovery copy. Keyed by buffer, so a renamed +-- buffer's real recovery key (written under the *old* path) is removed +-- too, not just the current path's. pmacs.hook.add("buffer.after-save", function() - local path = pmacs.editor.file_path() - if path then pcall(pmacs.autosave._discard, path) end + local buf = pmacs.window.buffer() + if buf then pcall(pmacs.autosave._discard_buffer, buf) end end) -- A final synchronous sweep on quit: async ticks stop after this, so a @@ -144,6 +156,14 @@ pmacs.command.define { if st == "stale" then warn = " [WARNING: file changed on disk since the autosave]" end + -- Pin to the exact BUFFER we started on, not merely its path: two + -- buffers can visit the same path (`pmacs.buffer.from_file` does not + -- dedup), so a path check alone could recover into the wrong one. + local origin_buf = pmacs.window.buffer() + if not origin_buf then + pmacs.editor.set_status("recover-file: no buffer") + return + end pmacs.minibuffer.read { prompt = "Recover from autosave?" .. warn .. " (yes/no): ", source = function() return { "yes", "no" } end, @@ -152,10 +172,10 @@ pmacs.command.define { pmacs.editor.set_status("recover-file: cancelled") return end - -- Pin to the buffer we started on: focus can drift while the - -- prompt is up, and recovering into the wrong buffer is - -- unrecoverable. - if pmacs.editor.file_path() ~= path then + -- Focus can drift while the prompt is up, and recovering into the + -- wrong buffer is unrecoverable. + local buf = pmacs.window.buffer() + if buf ~= origin_buf or pmacs.editor.file_path() ~= path then pmacs.editor.set_status("recover-file: buffer changed; aborted") return end @@ -164,7 +184,6 @@ pmacs.command.define { pmacs.editor.set_status("recover-file: recovery unreadable") return end - local buf = pmacs.window.buffer() buf:replace(0, buf:len(), bytes) -- The mutators notify windows and queue CRDT but do NOT fire -- `buffer.after-edit` --- that comes from dispatch_key's @@ -172,6 +191,10 @@ pmacs.command.define { -- returns before. Fire it so LSP didChange and the syntax -- reparse see the recovered contents. pmacs.hook.run("buffer.after-edit") + -- The crash data now lives in the buffer, so the copy is no + -- longer irreplaceable: claim it, which un-blocks autosave for + -- this path (Q#AS12). + pmacs.autosave._adopt(path) pmacs.editor.set_status("recovered from autosave --- save to keep it") end, } diff --git a/docs/autosave-recovery-framing.md b/docs/autosave-recovery-framing.md index 9a33ea7..aaec4e6 100644 --- a/docs/autosave-recovery-framing.md +++ b/docs/autosave-recovery-framing.md @@ -235,8 +235,11 @@ So the report is **pull-based and aggregated on the tick we already own** Recovery itself happens through an explicit command: -- **`recover-file`** — confirms via `minibuffer.read` (typed `yes`), then - `buf:replace(0, buf:len(), recovery_bytes)` on the active buffer, +- **`recover-file`** — confirms via `minibuffer.read` (typed `yes`), + **pins to the origin *buffer handle*, not merely its path** (finding: + `pmacs.buffer.from_file` does not dedup, so two buffers can visit one + path and a path check alone could recover into the wrong one), then + `buf:replace(0, buf:len(), recovery_bytes)`, **then explicitly `pmacs.hook.run("buffer.after-edit")`**. That last step is load-bearing (finding): the mutators only notify windows and queue CRDT, and `after-edit` is fired by `dispatch_key`'s post-command @@ -252,14 +255,45 @@ Recovery itself happens through an explicit command: This also sidesteps re-entrancy: no modal surface is opened from inside a hook fired by Rust. -### Q#AS7 — Cleanup lifecycle +### Q#AS12 — Never overwrite unclaimed crash data (the ownership rule) -- **`buffer.after-save`** → `discard(active path)`. A clean save means the - recovery copy is obsolete. (Hook exists, active-buffer, no args needed.) -- **Buffer killed** → `discard(path)`. There is no global kill hook, so - `after-load` registers a per-buffer `pmacs.buffer.on_removed(id, fn)` - closing over the path (captured then, since the buffer may be gone when - the callback runs). Killing a modified buffer is a deliberate discard. +The failure this closes (finding): you crash with unsaved work, reopen +the file, and start editing *before* running `recover-file`. The next +sweep writes the current buffer to the same key — **destroying the crash +copy**, which is precisely what autosave exists to protect. + +So the sweep tracks **ownership**. A per-session `owned` set records +which path hashes *this session* wrote or adopted. A recovery file +present at a key we do not own is unclaimed crash data: + +- the sweep **refuses to write** that buffer and counts it as `blocked`; +- `sweep()` surfaces *"autosave paused for N file(s) with unclaimed + recovery — M-x recover-file or M-x discard-recovery"*; +- `recover-file` **adopts** the copy once its contents are installed in + the buffer (the crash data now lives in the buffer, so the file is no + longer irreplaceable), and `discard-recovery` removes it. Either action + resumes normal autosave for that path. + +The trade is deliberate: while blocked, edits made *after* the reopen are +not autosaved — and the user is told so, every sweep. Losing the new +edits to a second crash is recoverable by retyping; losing the original +crash copy is not. + +### Q#AS7 — Cleanup lifecycle (keyed by buffer, not by a captured path) + +- **`buffer.after-save`** → `discard_buffer(active buffer)`. +- **Buffer killed** → `discard_buffer(id)`. There is no global kill hook, + so `after-load` registers a per-buffer `pmacs.buffer.on_removed`. +- Both go through **`discard_buffer(BufferId)`**, not a path captured at + load time (finding). It removes *both* the buffer's current-path key + and the key its last sweep actually **wrote** under — which differ + after a rename (an LSP `WorkspaceEdit` changes the path while the + `BufferId` stays). A path-captured callback would delete the wrong key + and leave the real recovery file behind. +- **Sweep-time GC** is the backstop: any cache entry whose `BufferId` has + left the registry has its recovery file deleted. This is what covers + argv **`[new file]`** buffers, which fire no `after-load` and so never + get a removal callback registered (finding). - **`editor.before-quit`** → one **final synchronous sweep**, then return nil (never veto). Async ticks stop after quit, so this must be a direct call. Result: quitting with unsaved changes leaves a recovery copy that @@ -311,7 +345,13 @@ So this PR makes autosave storage private: momentarily visible at `0644`. (A chmod-after-write leaves exactly that window.) Plain `save_atomic` delegates with `None`. - **`state::write_private(base, name, content)`** — creates the parent - with `DirBuilder::mode(0o700)` and writes the file `0600`. + with `DirBuilder::mode(0o700)` and writes the file `0600`. It also + **tightens a pre-existing lax `autosave/`** to `0700` (finding): the + birth-mode only applies to directories *that call* creates, so a + `0755` directory left by an older run would still leak recovery-file + names, sizes, and mtimes despite `0600` contents. It never re-modes + `base` itself — the state root is shared with history/recentf/desktop + and may predate us. - Recovery files use it; the `autosave/` directory is `0700`. - Unix-only (`PermissionsExt` / `DirBuilderExt` are safe under `#![forbid(unsafe_code)]`); on other platforms it degrades to today's @@ -400,10 +440,19 @@ commands, cleanup wiring) → tests. each recovery file is `0600` — asserted, not assumed. - Sweep skips clean buffers, scratch buffers, and buffers unchanged since the last sweep (no second write). +- **Unclaimed crash data is never overwritten (Q#AS12)**: session 1 + crashes with a recovery copy; session 2 reopens, edits, sweeps → + `(written, blocked) == (0, 1)` and the crash copy is byte-identical. + `_adopt` (what `recover-file` calls) or `_discard` resumes the sweep. - **Path change**: rename a buffer's path (`set_buffer_path`) without editing it → next sweep writes the new key **and** removes the old recovery file (the `(path_hash, revision)` cache). - `after-save` → recovery deleted. Kill buffer → recovery deleted. +- **Rename then save with no intervening sweep** → the recovery written + under the *old* key is removed (buffer-keyed cleanup, Q#AS7). +- **Killing a `[new file]` buffer** → the sweep-time GC removes its + recovery (no `after-load` fired, so no removal callback exists). +- **A pre-existing `0755` `autosave/` dir is tightened to `0700`.** - Open a file with a **`Fresh`** recovery → the aggregate report names it; buffer contents are still the on-disk ones (no silent substitution). diff --git a/src/autosave.rs b/src/autosave.rs index bfccb03..6c8e37e 100644 --- a/src/autosave.rs +++ b/src/autosave.rs @@ -18,7 +18,7 @@ //! Framing: docs/autosave-recovery-framing.md. use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use mlua::Lua; @@ -123,15 +123,33 @@ pub fn key_for(path: &Path) -> String { format!("autosave/{}", sha256_hex(&path.display().to_string())) } -/// Skip cache: `BufferId → (path_hash, revision)` (Q#AS8). -/// -/// Keyed on the **path hash as well as the revision**, not the revision -/// alone: a buffer keeps its `BufferId` across a path change (an LSP -/// `WorkspaceEdit` rename calls `set_buffer_path`), so a revision-only -/// cache would skip the write, never create the recovery file under the -/// new key, and orphan the old one. +/// Per-session autosave bookkeeping. #[derive(Default)] -pub struct AutosaveCache(RefCell>); +pub struct AutosaveCache(RefCell); + +#[derive(Default)] +struct CacheInner { + /// Skip cache: `BufferId → (path_hash, revision)` (Q#AS8). + /// + /// Keyed on the **path hash as well as the revision**, not the + /// revision alone: a buffer keeps its `BufferId` across a path change + /// (an LSP `WorkspaceEdit` rename calls `set_buffer_path`), so a + /// revision-only cache would skip the write, never create the + /// recovery file under the new key, and orphan the old one. It also + /// remembers *where a buffer's recovery currently lives*, which is + /// what makes cleanup work after a rename. + written: HashMap, + /// Path hashes whose recovery file **this session** wrote or has + /// adopted (Q#AS12). + /// + /// A recovery file we did *not* write is unclaimed crash data. If the + /// user reopens the file and starts editing, sweeping would overwrite + /// the crash copy with the current buffer — destroying exactly what + /// autosave exists to protect. So an unowned recovery file *blocks* + /// the sweep for that buffer until `recover-file` adopts it or + /// `discard-recovery` removes it. + owned: HashSet, +} /// Encode a header + contents into the one-file envelope. fn encode(header: &Header, contents: &[u8]) -> Result, String> { @@ -220,11 +238,15 @@ struct Pending { /// Unlike desktop-save this is **not** daemon-gated — autosave is /// per-buffer, not per-frontend, and a daemon holds the unsaved work. /// +/// Returns `(written, blocked)` — `blocked` counts buffers whose sweep +/// was refused because an **unclaimed** recovery file already sits at +/// their key (Q#AS12). +/// /// # Errors /// A state-write failure. Individual buffers never abort the pass. -pub fn sweep(lua: &Lua) -> Result { +pub fn sweep(lua: &Lua) -> Result<(usize, usize), String> { let Some(base) = base_dir(lua) else { - return Ok(0); + return Ok((0, 0)); }; let core = lua .app_data_ref::() @@ -235,6 +257,7 @@ pub fn sweep(lua: &Lua) -> Result { let mut writes: Vec = Vec::new(); let mut orphans: Vec = Vec::new(); let mut live: Vec = Vec::new(); + let mut blocked = 0usize; { let cache = lua .app_data_ref::() @@ -256,7 +279,7 @@ pub fn sweep(lua: &Lua) -> Result { let path_s = path.display().to_string(); let path_hash = sha256_hex(&path_s); let revision = buf.revision(); - if let Some((prev_hash, prev_rev)) = cache.get(&id) { + if let Some((prev_hash, prev_rev)) = cache.written.get(&id) { if prev_hash == &path_hash && *prev_rev == revision { continue; // unchanged since its last copy } @@ -265,6 +288,16 @@ pub fn sweep(lua: &Lua) -> Result { orphans.push(prev_hash.clone()); } } + // Never clobber unclaimed crash data (Q#AS12). A recovery file + // this session did not write is the crash copy the user has + // not recovered yet; overwriting it with the current buffer + // would destroy exactly what autosave protects. + if !cache.owned.contains(&path_hash) + && crate::state::exists(&base, &format!("autosave/{path_hash}")).unwrap_or(false) + { + blocked += 1; + continue; + } let len = buf.len(); let mut contents = vec![0u8; usize::try_from(len).unwrap_or(0)]; if len > 0 { @@ -291,8 +324,21 @@ pub fn sweep(lua: &Lua) -> Result { .app_data_ref::() .ok_or("no autosave cache")?; let mut cache = cache.0.borrow_mut(); - // Drop entries for buffers that no longer exist. - cache.retain(|id, _| live.contains(id)); + // GC: a buffer that left the registry (killed) takes its recovery + // copy with it. This is the backstop that covers `[new file]` + // buffers, which fire no `after-load` and so never get a + // per-buffer removal callback registered. + let dead: Vec<(BufferId, String)> = cache + .written + .iter() + .filter(|(id, _)| !live.contains(id)) + .map(|(id, (hash, _))| (*id, hash.clone())) + .collect(); + for (id, hash) in dead { + let _ = crate::state::remove(&base, &format!("autosave/{hash}")); + cache.written.remove(&id); + cache.owned.remove(&hash); + } for p in writes { let header = Header { version: AUTOSAVE_VERSION, @@ -302,11 +348,88 @@ pub fn sweep(lua: &Lua) -> Result { let bytes = encode(&header, &p.contents)?; crate::state::write_private(&base, &format!("autosave/{}", p.path_hash), &bytes) .map_err(|e| e.to_string())?; - cache.insert(p.id, (p.path_hash, p.revision)); + cache.owned.insert(p.path_hash.clone()); + cache.written.insert(p.id, (p.path_hash, p.revision)); written += 1; } } - Ok(written) + Ok((written, blocked)) +} + +/// Claim the recovery file at `path` for this session (Q#AS12), so +/// subsequent sweeps may overwrite it. Called by `recover-file` once its +/// contents are installed in the buffer — the crash data now lives in the +/// buffer, so the copy is no longer irreplaceable. +pub fn adopt(lua: &Lua, path: &Path) { + if let Some(cache) = lua.app_data_ref::() { + cache + .0 + .borrow_mut() + .owned + .insert(sha256_hex(&path.display().to_string())); + } +} + +/// Forget any claim on `path` (so a foreign recovery appearing there +/// later still blocks a sweep). +fn unown(lua: &Lua, path: &Path) { + if let Some(cache) = lua.app_data_ref::() { + cache + .0 + .borrow_mut() + .owned + .remove(&sha256_hex(&path.display().to_string())); + } +} + +/// Delete the recovery file for `path` and drop any claim on it. +pub fn discard_path(lua: &Lua, path: &Path) -> bool { + let Some(base) = base_dir(lua) else { + return false; + }; + unown(lua, path); + discard(&base, path) +} + +/// Retire the recovery copy of a specific **buffer** (Q#AS12). +/// +/// Keyed by `BufferId`, not by the path captured when the buffer loaded: +/// it removes both the buffer's *current* path key (if it is still live) +/// and the key its last recovery was actually **written** under. Those +/// differ after a rename — an LSP `WorkspaceEdit` changes the path while +/// the `BufferId` stays — and a path-captured callback would leave the +/// real recovery file behind. +pub fn discard_buffer(lua: &Lua, id: BufferId) { + let Some(base) = base_dir(lua) else { + return; + }; + let mut keys: Vec = Vec::new(); + // The key the last sweep actually wrote for this buffer. + if let Some(cache) = lua.app_data_ref::() + && let Some((hash, _)) = cache.0.borrow().written.get(&id) + { + keys.push(hash.clone()); + } + // The buffer's current path, which may have moved since that write. + if let Some(core) = lua.app_data_ref::() { + let c = core.borrow(); + let reg = c.registry.borrow(); + if let Ok(buf) = reg.get(id) + && let Some(p) = buf.file_path() + { + keys.push(sha256_hex(&p.display().to_string())); + } + } + for hash in &keys { + let _ = crate::state::remove(&base, &format!("autosave/{hash}")); + } + if let Some(cache) = lua.app_data_ref::() { + let mut cache = cache.0.borrow_mut(); + cache.written.remove(&id); + for hash in &keys { + cache.owned.remove(hash); + } + } } /// Every open file buffer that has a recovery file, with its status diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 6878d08..25dbfb1 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -1981,11 +1981,35 @@ fn install_autosave_module(lua: &Lua) -> mlua::Result { lua.set_app_data(crate::autosave::AutosaveCache::default()); let m = lua.create_table()?; + // _sweep() -> (written, blocked). `blocked` counts buffers whose + // sweep was refused because unclaimed crash data sits at their key. m.set( "_sweep", lua.create_function(|lua, ()| crate::autosave::sweep(lua).map_err(mlua::Error::external))?, )?; + // _adopt(path): claim a recovery file for this session, so later + // sweeps may overwrite it. `recover-file` calls this once the + // contents are safely installed in the buffer. + m.set( + "_adopt", + lua.create_function(|lua, path: String| { + crate::autosave::adopt(lua, std::path::Path::new(&path)); + Ok(()) + })?, + )?; + + // _discard_buffer(buf): retire a buffer's recovery copy by BufferId — + // removes both its current-path key and the key its last sweep wrote + // (they differ after a rename). + m.set( + "_discard_buffer", + lua.create_function(|lua, id: BufferIdLua| { + crate::autosave::discard_buffer(lua, id.0); + Ok(()) + })?, + )?; + m.set( "_status", lua.create_function(|lua, path: String| { @@ -2009,13 +2033,14 @@ fn install_autosave_module(lua: &Lua) -> mlua::Result
{ })?, )?; + // _discard(path): delete a recovery file and drop any claim on it. m.set( "_discard", lua.create_function(|lua, path: String| { - let Some(base) = lua.app_data_ref::().map(|d| d.0.clone()) else { - return Ok(false); - }; - Ok(crate::autosave::discard(&base, std::path::Path::new(&path))) + Ok(crate::autosave::discard_path( + lua, + std::path::Path::new(&path), + )) })?, )?; diff --git a/src/state.rs b/src/state.rs index 22ba299..0a783b7 100644 --- a/src/state.rs +++ b/src/state.rs @@ -204,6 +204,15 @@ pub fn write_private(base: &Path, name: &str, content: &[u8]) -> Result<(), Stat write_inner(base, name, content, Some(0o600)) } +/// True when a state file exists (no read, no parse). +/// +/// # Errors +/// Invalid key. +pub fn exists(base: &Path, name: &str) -> Result { + let path = resolve(base, name).map_err(StateError::Name)?; + Ok(path.exists()) +} + fn write_inner( base: &Path, name: &str, @@ -213,14 +222,25 @@ fn write_inner( let path = resolve(base, name).map_err(StateError::Name)?; if let Some(parent) = path.parent() { create_dir_all_with_mode(parent, mode.map(|_| 0o700)).map_err(StateError::Io)?; + // A directory *we* own beneath the state root (e.g. `autosave/`) + // must actually be `0700`, even if a previous run — or a user — + // created it laxer. Otherwise the mode only applies to the dirs + // this call happened to create, and a pre-existing `0755` + // `autosave/` would still leak recovery-file names, sizes, and + // mtimes despite the `0600` contents. + // + // Never re-mode `base` itself: the state root is a directory the + // user may already have, shared with history/recentf/desktop. + if mode.is_some() && parent != base { + enforce_dir_mode(parent, 0o700).map_err(StateError::Io)?; + } } crate::file_io::save_atomic_with_mode(&path, content, mode).map_err(StateError::Save)?; Ok(()) } -/// `create_dir_all`, optionally forcing the mode of directories this call -/// creates. An already-existing directory keeps its mode (we do not -/// re-mode a directory the user already has). +/// `create_dir_all`, birthing any directory this call creates at `mode` +/// (so it is never briefly world-readable). fn create_dir_all_with_mode(dir: &Path, mode: Option) -> std::io::Result<()> { #[cfg(unix)] if let Some(m) = mode { @@ -235,6 +255,22 @@ fn create_dir_all_with_mode(dir: &Path, mode: Option) -> std::io::Result<() std::fs::create_dir_all(dir) } +/// Tighten an existing directory to `mode` if it is laxer. No-op on +/// non-Unix, and cheap when already correct. +fn enforce_dir_mode(dir: &Path, mode: u32) -> std::io::Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + let current = std::fs::metadata(dir)?.permissions().mode() & 0o777; + if current != mode { + std::fs::set_permissions(dir, std::fs::Permissions::from_mode(mode))?; + } + } + #[cfg(not(unix))] + let _ = (dir, mode); + Ok(()) +} + /// Read a state file's raw bytes, or `Ok(None)` when it does not exist. /// /// [`read`] returns a `String` (`read_to_string`), which fails on diff --git a/tests/autosave_acceptance.rs b/tests/autosave_acceptance.rs index e420b39..16f1d82 100644 --- a/tests/autosave_acceptance.rs +++ b/tests/autosave_acceptance.rs @@ -48,9 +48,23 @@ fn eval(s: &EditorState, src: &str) -> T { /// Force a sweep; returns how many buffers were written. fn sweep(s: &EditorState) -> i64 { + let (written, _blocked): (i64, i64) = eval(s, "return pmacs.autosave.sweep()"); + written +} + +/// Force a sweep; returns `(written, blocked)`. +fn sweep2(s: &EditorState) -> (i64, i64) { eval(s, "return pmacs.autosave.sweep()") } +fn recovered(s: &EditorState, path: &str) -> Vec { + let b: mlua::String = eval( + s, + &format!("return pmacs.autosave._recover_bytes({path:?})"), + ); + b.as_bytes().to_vec() +} + fn status(s: &EditorState, path: &str) -> String { eval(s, &format!("return pmacs.autosave._status({path:?})")) } @@ -239,6 +253,142 @@ fn pending_aggregates_and_names_a_single_file() { std::fs::remove_dir_all(&dir).ok(); } +#[test] +fn sweep_never_overwrites_unclaimed_crash_recovery() { + let dir = fresh_state_dir(); + // Session 1 crashes with unsaved work: a recovery copy is on disk. + let s1 = editor(&dir); + let f = write_file(&dir, "a.txt", "on disk\n"); + open_and_dirty(&s1, &f, "CRASH WORK "); + assert_eq!(sweep(&s1), 1); + let crash_copy = recovered(&s1, &f); + assert_eq!(&crash_copy, b"CRASH WORK on disk\n"); + + // Session 2 reopens the file (on-disk contents) and edits it BEFORE + // running recover-file. Sweeping must NOT clobber the crash copy. + let s2 = editor(&dir); + exec(&s2, &format!("pmacs.buffer.find_or_open({f:?})")); + exec(&s2, "pmacs.window.buffer():insert(0, 'new edits ')"); + let (written, blocked) = sweep2(&s2); + assert_eq!(written, 0, "must not write over unclaimed crash data"); + assert_eq!(blocked, 1, "the sweep is blocked and reported"); + assert_eq!( + recovered(&s2, &f), + crash_copy, + "the crash recovery survives intact" + ); + assert_eq!(status(&s2, &f), "fresh", "still offered to the user"); + + // Once recover-file adopts it, autosave resumes for that path. + exec(&s2, &format!("pmacs.autosave._adopt({f:?})")); + let (written, blocked) = sweep2(&s2); + assert_eq!((written, blocked), (1, 0), "adopted → sweeps again"); + assert_eq!(recovered(&s2, &f), b"new edits on disk\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn discarding_an_unclaimed_recovery_unblocks_the_sweep() { + let dir = fresh_state_dir(); + let s1 = editor(&dir); + let f = write_file(&dir, "a.txt", "on disk\n"); + open_and_dirty(&s1, &f, "crash "); + assert_eq!(sweep(&s1), 1); + + let s2 = editor(&dir); + exec(&s2, &format!("pmacs.buffer.find_or_open({f:?})")); + exec(&s2, "pmacs.window.buffer():insert(0, 'mine ')"); + assert_eq!(sweep2(&s2), (0, 1), "blocked"); + + exec(&s2, &format!("pmacs.autosave._discard({f:?})")); + assert_eq!(sweep2(&s2), (1, 0), "discarded → sweeps again"); + assert_eq!(recovered(&s2, &f), b"mine on disk\n"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn killing_a_new_file_buffer_gcs_its_recovery() { + let dir = fresh_state_dir(); + let s = editor(&dir); + // A `[new file]` fires no after-load, so no per-buffer removal + // callback is registered — the sweep-time GC is the backstop. + let missing = dir.join("draft.txt"); + exec( + &s, + "_G.nb = pmacs.buffer.create('draft.txt'); pmacs.window.switch_buffer(_G.nb)", + ); + { + let id = s.core.borrow().active_buffer_id(); + s.core + .borrow_mut() + .set_buffer_path(id, Some(missing.clone())); + } + exec(&s, "pmacs.window.buffer():insert(0, 'draft')"); + let p = missing.display().to_string(); + assert_eq!(sweep(&s), 1); + assert_eq!(status(&s, &p), "fresh"); + + exec(&s, "pmacs.buffer.kill(_G.nb)"); + // The next sweep GCs the dead buffer's recovery copy. + sweep(&s); + assert_eq!( + status(&s, &p), + "none", + "killed new-file buffer is cleaned up" + ); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn saving_after_a_rename_removes_the_recovery_written_under_the_old_path() { + let dir = fresh_state_dir(); + let s = editor(&dir); + let old = write_file(&dir, "old.txt", "body\n"); + let new = dir.join("new.txt").display().to_string(); + open_and_dirty(&s, &old, "dirty "); + assert_eq!(sweep(&s), 1, "recovery written under the OLD key"); + + // Rename, then save — without an intervening sweep. A path-captured + // cleanup would delete the new key and leave the old one behind. + std::fs::rename(&old, &new).unwrap(); + { + let id = s.core.borrow().active_buffer_id(); + s.core + .borrow_mut() + .set_buffer_path(id, Some(PathBuf::from(&new))); + } + exec(&s, "pmacs.command.invoke('buffer.save')"); + assert_eq!(status(&s, &old), "none", "old key removed"); + assert_eq!(status(&s, &new), "none", "new key removed"); + std::fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn a_pre_existing_lax_autosave_dir_is_tightened() { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + let dir = fresh_state_dir(); + // Someone (an older pmacs, or the user) left autosave/ at 0755. + let autosave_dir = dir.join("autosave"); + std::fs::create_dir_all(&autosave_dir).unwrap(); + std::fs::set_permissions(&autosave_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let s = editor(&dir); + let f = write_file(&dir, "a.txt", "body\n"); + open_and_dirty(&s, &f, "secret "); + assert_eq!(sweep(&s), 1); + + let dmode = std::fs::metadata(&autosave_dir) + .unwrap() + .permissions() + .mode() + & 0o777; + assert_eq!(dmode, 0o700, "a lax autosave dir is tightened, not left"); + std::fs::remove_dir_all(&dir).ok(); + } +} + #[test] fn tick_reports_recoveries_once_aggregated() { let dir = fresh_state_dir();