neuwirth
/
pmacs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(persistence): never clobber unclaimed crash data; buffer-keyed cleanup 

Addresses the PR #100 review.

- HIGH data loss: sweep could overwrite an existing crash recovery before
  the user ran recover-file. Reopen a file after a crash, edit it, and the
  next autosave wrote the current buffer over the recovery key --- losing
  exactly what autosave exists to protect. New ownership rule (Q#AS12): a
  per-session `owned` set records which path hashes THIS session wrote or
  adopted. A recovery file at a key we do not own is unclaimed crash data;
  the sweep refuses to write that buffer, counts it `blocked`, and says so
  ("autosave paused for N file(s) with unclaimed recovery"). recover-file
  ADOPTS the copy once its contents are in the buffer; discard-recovery
  removes it. Either resumes normal autosave. sweep() now returns
  (written, blocked).

- MEDIUM cleanup missed paths autosave can write. Kill/save cleanup now
  goes through `discard_buffer(BufferId)`, which removes BOTH the buffer's
  current-path key and the key its last sweep actually wrote (they differ
  after a rename --- an LSP WorkspaceEdit changes the path while the
  BufferId stays; a path-captured callback deleted the wrong key). And a
  sweep-time GC deletes the recovery of any buffer that left the registry,
  which is the backstop for argv `[new file]` buffers: they fire no
  after-load, so no removal callback is ever registered for them.

- LOW/MEDIUM recover-file pinned only on the active path. Two buffers can
  visit one path (pmacs.buffer.from_file does not dedup), so focus drift
  could recover into the wrong buffer. It now captures and compares the
  origin buffer handle as well as the path.

- LOW write_private left a pre-existing lax autosave/ directory alone. The
  birth-mode only applies to dirs that call creates, so a 0755 autosave/
  from an older run still leaked recovery-file names, sizes, and mtimes
  despite 0600 contents. It is now tightened to 0700 --- but never `base`
  itself, which is shared with history/recentf/desktop and may predate us.
  New `state::exists` (an existence check, no read) backs the ownership
  gate.

Tests (autosave_acceptance now 20): sweep_never_overwrites_unclaimed_
crash_recovery (blocked, crash copy byte-identical, adopt resumes),
discarding_an_unclaimed_recovery_unblocks_the_sweep,
killing_a_new_file_buffer_gcs_its_recovery,
saving_after_a_rename_removes_the_recovery_written_under_the_old_path,
a_pre_existing_lax_autosave_dir_is_tightened.

Gates: fmt + workspace clippy clean; lib 1499; crdt 1670; autosave 20;
desktop 11; persistence 5; m4 90; m7_8 5; GPU 58; git diff --check clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-07-09 10:52:10 -04:00
parent ec42526652
commit 1205329c34
6 changed files with 457 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
builtin/runtime/autosave.lua
View File

@ -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,
}

69
docs/autosave-recovery-framing.md
View File

@ -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).

155
src/autosave.rs
View File

@ -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<HashMap<BufferId, (String, u64)>>);
pub struct AutosaveCache(RefCell<CacheInner>);
#[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<BufferId, (String, u64)>,
/// 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<String>,
}
/// Encode a header + contents into the one-file envelope.
fn encode(header: &Header, contents: &[u8]) -> Result<Vec<u8>, 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<usize, String> {
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::<SharedCore>()
@ -235,6 +257,7 @@ pub fn sweep(lua: &Lua) -> Result<usize, String> {
let mut writes: Vec<Pending> = Vec::new();
let mut orphans: Vec<String> = Vec::new();
let mut live: Vec<BufferId> = Vec::new();
let mut blocked = 0usize;
{
let cache = lua
.app_data_ref::<AutosaveCache>()
@ -256,7 +279,7 @@ pub fn sweep(lua: &Lua) -> Result<usize, String> {
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<usize, String> {
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<usize, String> {
.app_data_ref::<AutosaveCache>()
.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<usize, String> {
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::<AutosaveCache>() {
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::<AutosaveCache>() {
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<String> = Vec::new();
// The key the last sweep actually wrote for this buffer.
if let Some(cache) = lua.app_data_ref::<AutosaveCache>()
&& 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::<SharedCore>() {
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::<AutosaveCache>() {
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

33
src/lua_bindings/mod.rs
View File

@ -1981,11 +1981,35 @@ fn install_autosave_module(lua: &Lua) -> mlua::Result<Table> {
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<Table> {
})?,
)?;
// _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::<StateDir>().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),
))
})?,
)?;

42
src/state.rs
View File

@ -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<bool, StateError> {
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<u32>) -> std::io::Result<()> {
#[cfg(unix)]
if let Some(m) = mode {
@ -235,6 +255,22 @@ fn create_dir_all_with_mode(dir: &Path, mode: Option<u32>) -> 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

150
tests/autosave_acceptance.rs
View File

@ -48,9 +48,23 @@ fn eval<T: mlua::FromLuaMulti>(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<u8> {
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();