Fix #1131: roll the save lock back to v0.45.0 plus only the #1108 Ubuntu 26.04 fix

changelog.d/1119.fixed.md

@@ -1,18 +1 @@
-- Fixed: `_lock_read_pid_timeout` (`scripts/lib-lock.sh`) treated a FAST,
-  ordinary read failure (permission denied, I/O error, a cloud-sync
-  placeholder that errors rather than hangs) exactly like a successful read
-  of a genuinely empty pid file, a regression from the #1101/#1103 lock-
-  staleness fix (v0.45.1). Because a confirmed-empty pid file is reclaimed
-  after only `REMEMBER_LOCK_ADOPT_AFTER` (30s) rather than
-  `REMEMBER_LOCK_MAX_HOLD_SECONDS` (1800s), this let a second process steal
-  `save.lock`/`consolidation.lock`/`staging.lock` away from a holder that
-  was still alive, 60x sooner than any other takeover path allows (#1119).
-  A fast read failure is now treated the same as a read that never
-  completed: refused on its own, but still recoverable through the
-  existing age-gated takeover once the lock directory is older than
-  `REMEMBER_LOCK_MAX_HOLD_SECONDS`.
-- Corrects the v0.45.1 changelog entry for #1101/#1103, which claimed a
-  confirmed-empty pid file is "reclaimed too, exactly as it already was for
-  the no-pid-file-at-all case" -- not true for a pid file that reads empty
-  because the read failed fast rather than because it genuinely is empty;
-  v0.45.0 never stole a lock in that case either (#1119).
+- Fixed: in v0.45.1, if a save could not read its lock file (a permissions error, or a cloud-sync placeholder that answers with an error), another save could take over the lock while the first one was still running, so two saves could run at the same time. Putting the save lock back the way it worked in v0.45.0 removes this (see the removal note in this release). (#1119)

changelog.d/1122.fixed.md

@@ -1,19 +1 @@
-- Fixed: `_lock_try_steal` (`scripts/lib-lock.sh`) re-reads the pid file it
-  just renamed aside to tell apart "still the same stuck placeholder" (safe
-  to steal) from "a faster contender's own write already recreated the pid
-  file in the gap between this contender's own age check and its rename"
-  (unsafe -- must hand back). #1119/#1120 (v0.45.1) made every read failure
-  at this re-read report identically, whether a real timeout or a fast
-  EACCES/EIO/cloud-sync refusal, so a fast-failing re-read of a genuine
-  winner's brand-new pid was read exactly like a confirmed-still-stuck
-  placeholder and fell through to the noclobber write below it, clobbering
-  the winner's `save.lock`/`consolidation.lock`/`staging.lock` and
-  producing two simultaneous holders -- the same class #1119/#1120 shipped
-  as a non-blocking trap.d fragment one release earlier and then had to
-  patch immediately as a regression. The re-read now also compares the
-  renamed file's inode against what was captured just before the rename:
-  a match is the same file whose first read already failed too, safe to
-  steal; a mismatch, or either lookup coming back empty, hands back rather
-  than guess -- `stat` needs no read permission on the file's content,
-  unlike the re-read itself, so this works even when neither pid file is
-  readable (#1122).
+- Fixed: a follow-up attempt to repair the v0.45.1 save-lock problem, never released, still let two saves run at once in a rarer timing case. It is dropped together with the rest of that change by the rollback in this release, so there is nothing to update for it. (#1122)

changelog.d/1131.removed.md

@@ -0,0 +1,7 @@
+- Removed: the save-lock recovery added in the previous release (v0.45.1). It was meant to unstick a save lock left behind by a hung save, or by a lock file that a cloud-sync folder (OneDrive, iCloud, Dropbox) had turned into an online-only placeholder. Reviews found it could let two saves run at the same time, which is worse than the problem it fixed, so the save lock now works exactly as it did in v0.45.0. The Ubuntu 26.04 fix that stops two saves running at once there (#1108) stays.
+
+  What this brings back, until the recovery is redesigned as a separate change: a lock left by a save that hung can stay stuck (#1101, reopened), and if your `.remember` folder is in a cloud-sync folder, reading an online-only lock file can make a save wait until it times out (#1103, reopened).
+
+  The two settings that came with the recovery, `REMEMBER_LOCK_MAX_HOLD_SECONDS` and `REMEMBER_LOCK_PID_READ_TIMEOUT`, are gone. If you set either one, it is now ignored; you can delete it. (#1131)
+
+- Compatibility: compatible - nothing that worked in v0.45.0 stops working; the two removed settings are ignored rather than rejected, and every lock behaviour is the v0.45.0 one.

scripts/save-session.sh

@@ -1211,13 +1211,7 @@ fi
 # does not end in .md so neither today-*.md nor session-start injection can see
 # it — but it would accumulate forever, so sweep first. Safe: this block is the
 # only thing that creates these, it runs under save.lock, and NDC's sweep uses a
-# different glob (now.md.ndc-*). "Runs under save.lock" assumed, until #1101,
-# that the holder here still owned the lock throughout -- #1101 added a
-# takeover of a live-but-wedged holder (lib-lock.sh's _LOCK_MAX_HOLD_SECONDS
-# path), so that assumption no longer holds unconditionally; this block does
-# not re-check ownership before writing, a gap raised in the v0.45.0 release
-# audit (PR #1118) and left open here -- #1119 only corrects this comment,
-# it does not close the gap.
+# different glob (now.md.ndc-*).
 append_failed() {
     rm -f "$APPEND_TMP" 2>/dev/null
     log "write" "ERROR: cannot write now.md -- $1"
@@ -1593,18 +1587,9 @@ if [ "$RUN_NDC" = true ]; then
                     # deadlock (lock_acquire spins to a deadline and returns 1)
                     # and costs nothing: nothing waits on this subshell.
                     #
-                    # #1101 added exactly the takeover this comment used to
-                    # say could not happen: a live-but-wedged holder is reclaimed
-                    # once the lock directory is older than
-                    # _LOCK_MAX_HOLD_SECONDS (lib-lock.sh), independent of
-                    # whether the original holder is still making progress.
-                    # Losing the wait therefore no longer proves someone is
-                    # genuinely holding it -- it may mean a takeover happened
-                    # instead. Neither this acquire nor the commit below
-                    # re-validates ownership once acquired -- a gap raised in
-                    # the v0.45.0 release audit (PR #1118) and left open here;
-                    # #1119 only corrects this comment, it does not close the
-                    # gap.
+                    # It cannot steal the lock from a live save either — steal
+                    # needs a dead pid, adoption needs no pid at all — so losing
+                    # the wait means someone is genuinely holding it.
                     if [ "$NDC_STAGED" = true ] && lock_acquire "$LOCK_DIR" "$NDC_COMMIT_LOCK_TIMEOUT"; then
                         # Re-read the size under the lock. The offset stays valid as
                         # the START of the kept range no matter what happened during