Commit Graph

5 Commits

Author SHA1 Message Date
Levi Neuwirth c80e00e799 fix(persistence): adopt clears the old owner's skip cache; failing sweeps are loud
Addresses the PR #100 review round 4.

- MEDIUM stale skip-cache entry after a slot transfer. adopt() set
  owner[hash] = new buffer but left the previous owner's `written` entry
  pointing at the same hash, breaking the invariant
  `written[id] => owner[hash] == id`. Repro: A and B are duplicate buffers
  on one path; A owns the slot; B adopts (recover-file); B is killed
  without saving, which frees the slot and deletes the file. A is still
  dirty, but its stale written[A] = (hash, revA) makes the next sweep call
  it "unchanged since its last copy" --- silently unprotected until its
  next edit. adopt() now drops any other buffer's written entry for that
  hash. Verified the new test fails without the fix (sweep writes 0).

- MEDIUM autosave write failures were swallowed. write_private can fail
  (ENOSPC, a permission change, a clobbered state dir), but the tick and
  before-quit paths did `pcall(sweep)` and dropped the error. For a
  data-protection feature that is the worst failure mode: the user keeps
  working, believing edits are captured, while nothing is written. Both
  paths now go through a reporting wrapper --- status line "autosave
  FAILED: ... --- your work is NOT being protected" on every failing sweep,
  each distinct fault logged once via pmacs.error. The quit path reports
  too (a failure there means the quit is about to discard work that was
  never written anywhere) and still never vetoes.

Tests (autosave_acceptance now 29):
adopting_clears_the_previous_owners_stale_skip_cache,
a_failing_sweep_is_reported_not_swallowed (plants a regular file where
autosave/ must be a directory, standing in for ENOSPC).

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 12:06:49 -04:00
Levi Neuwirth e173f61a61 fix(persistence): one buffer owns a path's recovery slot (Q#AS13)
Addresses the PR #100 review round 3.

pmacs.buffer.from_file does not dedup, so two buffers can visit one path.
Ownership was tracked as a path-wide `owned: HashSet<path_hash>`, which
made the duplicate case silently corrupting: both dirty buffers queued a
write to autosave/<same hash>, the later write won on disk, and BOTH were
recorded in `written` --- so the loser skipped future sweeps while its
contents were unrecoverable. The path-wide set also let either buffer's
save/kill retire the other's recovery.

A recovery file must stay keyed by path (a later session knows only
paths, never old BufferIds), so two divergent buffers cannot both be
protected under one key. Ownership is now `owner: path_hash -> BufferId`:

- the first modified buffer to reach a free slot claims it, including
  within a single pass (the write loop updates `owner`, so the gather
  loop tracks slots queued this pass --- otherwise two duplicates both
  queue a write);
- any other buffer on that path is counted `conflicted` and reported
  ("autosave paused for N buffer(s): another buffer is visiting the same
  file"), never silently mis-protected. It records no `written` entry, so
  it re-attempts each sweep instead of believing itself saved;
- `discard_buffer` (save/kill) retires ONLY slots this buffer owns, which
  now enforces both invariants at once: an unowned slot is unclaimed
  crash data (Q#AS12), and a slot owned by another buffer is that
  buffer's recovery;
- saving or killing the owner releases the slot; the duplicate claims it
  on the next sweep;
- `recover-file` adopting into a buffer makes that buffer the owner --- the
  file's contents are now its contents, and the previous owner truthfully
  becomes conflicted.

sweep() now returns (written, blocked, conflicted). Its gather phase is
extracted into `gather()` (clippy too-many-lines).

This is honest rather than clever: pmacs cannot protect two divergent
buffers over one file, and now says so instead of pretending.

Tests (autosave_acceptance now 27):
duplicate_buffers_on_one_path_conflict_instead_of_corrupting (owner's
copy on disk; the dup never wins the slot by editing),
a_duplicate_buffers_save_does_not_retire_the_owners_recovery,
killing_the_owner_frees_the_slot_for_the_duplicate.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 11:36:55 -04:00
Levi Neuwirth b8a296f0a3 fix(persistence): only recover/discard may release unclaimed crash data
Addresses the PR #100 review round 2. Q#AS12's ownership rule guarded the
sweep but not the RELEASE paths, so three doors were still open.

The rule is now total: exactly two things may release an unclaimed
recovery file --- recover-file (which adopts it) and discard-recovery
(explicit user intent). Not a sweep, not a save, not a kill.

- HIGH: buffer.after-save called _discard_buffer unconditionally, which
  removed the live buffer's current-path key without checking ownership.
  Repro: session 1 autosaves and crashes; session 2 opens the file, does
  not recover, then saves --- the crash artifact was deleted. Same door
  was open on kill. discard_buffer now removes ONLY keys this session
  owns. The unclaimed copy survives (reported Stale, so never
  auto-offered, but still recoverable/discardable). The on-disk file holds
  the new work; the crash copy holds work never written anywhere, so
  deleting it was the same data loss by a different door.

- MEDIUM/LOW: _adopt only recorded the path in `owned`, not an
  association with the buffer. A removal callback fires after the buffer
  has left the registry, so discard_buffer had no path to read and no
  `written` entry to fall back on --- recover-then-kill leaked the copy
  and it was offered again. adopt now takes the BUFFER and records a
  `written` entry at the revision whose contents the file holds. That is
  correct twice over: the skip cache declines to rewrite an identical
  copy, and a kill can find and retire it.

- LOW: _discard(path) removed the file and unowned the hash but left
  matching `written` entries, so a still-dirty buffer hit the unchanged
  (path_hash, revision) fast path and went unprotected until its next
  edit. discard_path now clears those entries; the next sweep re-protects
  immediately.

Tests (autosave_acceptance now 24):
saving_without_recovering_preserves_unclaimed_crash_data,
killing_without_recovering_preserves_unclaimed_crash_data,
recover_then_kill_retires_the_adopted_recovery,
discard_recovery_lets_the_next_sweep_reprotect_immediately.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 11:08:33 -04:00
Levi Neuwirth 1205329c34 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>
2026-07-09 10:52:10 -04:00
Levi Neuwirth ec42526652 feat(persistence): autosave + crash recovery (Arc 3 phase 3)
Framing: docs/autosave-recovery-framing.md (Q#AS1-11). Closes the
persistence arc. Every modified file buffer is periodically written to a
private recovery copy; if pmacs dies, the next session says so and
`M-x recover-file` installs it. Emacs's auto-save-mode + recover-file.

Hybrid, forced by the same two gaps as phase 2: Lua has no per-buffer
path getter and FileMeta is neither Lua-visible nor serde. Rust owns the
sweep and the external-change guard; Lua owns cadence, config, and UX.

src/autosave.rs (new):
- One atomic envelope per recovery: a JSON header line + `\n` + raw
  buffer bytes. Split at the FIRST newline, so contents may hold newlines
  and non-UTF-8. A crash can never leave a torn header/contents pair.
- `origin` is NULLABLE: a `[new file]` buffer (a path with nothing on
  disk) has no FileMeta, and its unsaved contents are exactly the work
  most worth recovering.
- status(): Fresh / Stale / Corrupt / None. Only Fresh is announced;
  Stale (file changed, deleted, or created underneath us) is never
  auto-offered; Corrupt is typed, quiet, and discardable.
- sweep(): all modified file buffers, skipping clean/scratch and those
  unchanged since their last copy. The skip cache is keyed
  BufferId -> (path_hash, revision), not revision alone: a buffer keeps
  its BufferId across a path change (LSP WorkspaceEdit rename), so a
  revision-only cache would skip the write and orphan the old key.
- pending(): enumerates ALL open file buffers in Rust, which is what
  covers argv `[new file]` buffers -- they fire no hook at all.

Private storage (Q#AS11, a precondition for default-on): autosave stores
unsaved FILE CONTENTS, not metadata. New `file_io::save_atomic_with_mode`
sets the temp's mode BEFORE the rename (a chmod-after-write leaves a
window where the file is 0644), and `state::write_private` creates the
dir 0700 and the file 0600. Plus `state::read_bytes` (state::read is
read_to_string, which non-UTF-8 buffer contents would fail).

builtin/runtime/autosave.lua:
- Cadence is `process.after-tick` + monotonic_ms, NOT workers.sleep: a
  long sleep parks one of only `available_parallelism - 1` pool threads,
  and re-reading the interval each tick makes it live-reconfigurable.
- pmacs.autosave.interval_ms([ms]) -- validated getter/setter following
  the async_config.frame_target_ms shape. Default 30000, floor 1000.
  pmacs.autosave.enable(on). On by default.
- Notify, never prompt: `after-load` only raises a flag; the tick emits
  ONE aggregate message ("3 files have autosave recovery"). A modal
  prompt from after-load would stack N modals during a desktop restore.
- recover-file confirms, pins to the origin buffer, replaces contents,
  then explicitly fires `buffer.after-edit` -- the mutators only notify
  windows and queue CRDT, and after-edit comes from dispatch_key's
  post-command check, which the minibuffer shadow returns before. Without
  the explicit fire, LSP didChange and the syntax reparse never see the
  recovery. discard-recovery deletes a copy (including a Corrupt one).
- Cleanup: after-save discards; per-buffer on_removed discards on kill
  (there is no global kill hook); before-quit does one final synchronous
  sweep and never vetoes.

src/hash.rs (new): one pub(crate) sha256_hex, shared by desktop, autosave,
and packages::fetcher -- which had two private duplicates (Q#AS9).

Not daemon-gated (unlike desktop-save): autosave is per-buffer, not
per-frontend, and a daemon holds the unsaved work.

Tests: 8 autosave units + 13 state/hash units + tests/autosave_acceptance
(15): sweep round-trip, non-UTF-8 envelope, [new file] null-origin
Fresh->Stale, 0600/0700 perms, skip clean/scratch/unchanged, path-change
rewrites new key + discards old, save/kill cleanup, Stale not offered,
Corrupt typed+quiet+discardable, recover-file installs + fires after-edit
+ leaves modified, tick aggregation (3 loads -> 1 message, no repeat),
single-file naming, interval validation + live change, enable gate,
before-quit sweeps without vetoing.

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-09 10:24:08 -04:00