fix(lease): one directory lock for publish, prune and reclaim; reclaim rereads the record judged stale, so a renewal is never deleted or hidden

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-lease

@@ -80,31 +80,44 @@ valid_identity() {
     *) return 1 ;;
   esac
 }
-# state FILE PID — alive | dead | stale. Stale needs proof: two well-formed
-# identities from the same backend that differ. An identity that cannot be read
-# now, a malformed record or a backend change keeps the lease (never reclaim a
-# live session on missing evidence).
+# state PID RECORD AGE — alive | dead | stale for a lease whose record (first
+# line) and age were read once by the caller. Stale needs proof: two
+# well-formed identities from the same backend that differ. An identity that
+# cannot be read now, a malformed record or a backend change keeps the lease
+# (never reclaim a live session on missing evidence).
 state() {
-  alive "$2" || { echo dead; return; }
-  _rec=$(head -1 "$1" 2>/dev/null)
-  if [ -z "$_rec" ]; then
-    [ "$(age_min "$1")" -ge 120 ] && echo stale || echo alive
+  alive "$1" || { echo dead; return; }
+  if [ -z "$2" ]; then
+    [ "$3" -ge 120 ] && echo stale || echo alive
     return
   fi
-  _now=$(identity "$2")
-  if valid_identity "$_rec" && [ -n "$_now" ] && [ "${_rec%% *}" = "${_now%% *}" ] && [ "$_rec" != "$_now" ]; then echo stale
+  _now=$(identity "$1")
+  if valid_identity "$2" && [ -n "$_now" ] && [ "${2%% *}" = "${_now%% *}" ] && [ "$2" != "$_now" ]; then echo stale
   else echo alive
   fi
 }
-# reclaim FILE RECORD — remove a stale lease without losing a concurrent renewal:
-# rename it aside atomically, and put it back (hard link: only if no newer lease
-# was written meanwhile) when what was renamed is not the record judged stale.
+# The lease directory's lock: touch's publish and prune, and reclaim's
+# reread-and-delete, run under it, so a reclaim never deletes a renewal and a
+# lease never disappears while it is live. mkdir is atomic everywhere; a lock
+# left by a killed process is broken once it is a minute old.
+lock() {
+  _tries=0
+  until mkdir "$DIR/.lock" 2>/dev/null; do
+    _tries=$((_tries + 1))
+    if [ "$_tries" -ge 50 ]; then
+      [ -n "$(find "$DIR/.lock" -maxdepth 0 -mmin +1 2>/dev/null)" ] && rmdir "$DIR/.lock" 2>/dev/null && continue
+      return 1
+    fi
+    sleep 0.1
+  done
+}
+unlock() { rmdir "$DIR/.lock" 2>/dev/null; }
+# reclaim FILE RECORD — delete a stale lease only if, under the lock, it still
+# holds the record judged stale; a renewal since then keeps it.
 reclaim() {
-  _aside="${1%/*}/.reclaim.${1##*/}.$$"
-  mv -f "$1" "$_aside" 2>/dev/null || return 1
-  if [ "$(head -1 "$_aside" 2>/dev/null)" = "$2" ]; then rm -f "$_aside"; return 0; fi
-  ln "$_aside" "$1" 2>/dev/null
-  rm -f "$_aside"
+  lock || return 1
+  if [ "$(head -1 "$1" 2>/dev/null)" = "$2" ]; then rm -f "$1"; unlock; return 0; fi
+  unlock
   return 1
 }
 age_min() { # FILE — minutes since its mtime
@@ -116,15 +129,17 @@ case "$VERB" in
   touch)
     [ -n "$PID" ] || PID=$(gstack_harness_pid "${FROM:-$PPID}")
     mkdir -p "$DIR" || exit 2
-    _tmp="$DIR/.$PID.$$"
-    identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; exit 2; }
-    find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null
+    _tmp="$DIR/.$PID.$$" _locked=1
+    lock && _locked=0
+    identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; [ "$_locked" = 0 ] && unlock; exit 2; }
+    [ "$_locked" = 0 ] && { find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null; unlock; }
     exit 0 ;;
   list)
     for f in "$DIR"/*; do
       [ -f "$f" ] || continue
       p=${f##*/}
-      echo "$p $(age_min "$f")m $(state "$f" "$p")"
+      _age=$(age_min "$f")
+      echo "$p ${_age}m $(state "$p" "$(head -1 "$f" 2>/dev/null)" "$_age")"
     done
     exit 0 ;;
 esac
@@ -143,10 +158,10 @@ for f in "$DIR"/*; do
   [ -f "$f" ] || continue
   p=${f##*/}
   case "$ANCESTRY" in *" $p "*) continue ;; esac
-  case "$(state "$f" "$p")" in
+  _rec=$(head -1 "$f" 2>/dev/null) _age=$(age_min "$f")
+  case "$(state "$p" "$_rec" "$_age")" in
     dead) continue ;;
     stale)
-      _rec=$(head -1 "$f" 2>/dev/null) _age=$(age_min "$f")
       if reclaim "$f" "$_rec"; then
         echo "gstack-lease: reclaimed stale lease $p (${_age}m; recorded '$_rec', pid $p is now $(ps -o comm= -p "$p" 2>/dev/null | head -1) '$(identity "$p")')" >&2
         continue

docs/capy-install.md

@@ -88,9 +88,11 @@ process, or an identity-less lease from an older gstack past 120 minutes, is
 stale: `bin/gstack-lease live` reclaims it (`gstack-lease: reclaimed stale
 lease …`). Stale needs proof: two well-formed identities from the same backend
 that differ (`ps` is read under `LC_ALL=C TZ=UTC0`). An identity that cannot be
-read, a malformed record or a backend change keeps the lease. Reclaiming renames
-the lease aside first and restores it when a renewal landed meanwhile, and
-`touch` publishes by rename, so a reader never sees a half-written record. `bin/gstack-lease list` shows `alive|dead|stale`, and
+read, a malformed record or a backend change keeps the lease. `touch` (publish
+by rename, prune) and reclaim (reread, then delete only the record judged
+stale) run under one `mkdir` lock in the lease directory, so a renewal is never
+deleted and a live lease never disappears from a concurrent scan; a lock left
+by a killed process is broken after a minute. `bin/gstack-lease list` shows `alive|dead|stale`, and
 `GSTACK_UPGRADE_FORCE=1` overrides.
 
 ## What the installer does

test/gstack-lease.test.ts

@@ -150,7 +150,7 @@ describe('gstack-lease', () => {
     expect(r.stderr).not.toContain('reclaimed');
   });
 
-  test('a renewal that lands while a stale lease is being reclaimed survives (rename aside, then restore)', () => {
+  test('a renewal that lands between the stale verdict and the reclaim survives: reclaim rereads under the lock', () => {
     const { dir, leases } = checkout();
     const holder = sleeper();
     const file = path.join(leases, String(holder));
@@ -159,8 +159,8 @@ describe('gstack-lease', () => {
     fs.writeFileSync(file, otherStart(fresh.trim()) + '\n');
     const renewed = path.join(dir, 'renewed');
     fs.writeFileSync(renewed, fresh);
-    // The first rename-aside of the lease is preceded by the holder's renewal, as if touch ran between the stale verdict and the rename.
-    const racing = bashEnv(dir, `mv() { case "$*" in *".reclaim."*) command cp "${renewed}" "${file}" ;; esac; command mv "$@"; }\n`);
+    // The holder renews just before reclaim takes the lock, as if touch ran after the stale verdict.
+    const racing = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command mkdir "$@"; }\n`);
     const r = run(['live', '--root', dir], { BASH_ENV: racing });
     expect(r.status, r.stdout + r.stderr).toBe(0);
     expect(r.stderr).not.toContain('reclaimed');
@@ -168,6 +168,23 @@ describe('gstack-lease', () => {
     expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]);
   });
 
+  test('touch and reclaim take the directory lock; a lock a killed process left behind is broken after a minute', () => {
+    const { dir, leases } = checkout();
+    fs.mkdirSync(path.join(leases, '.lock'), { recursive: true });
+    const old = new Date(Date.now() - 5 * 60_000);
+    fs.utimesSync(path.join(leases, '.lock'), old, old);
+    const holder = sleeper();
+    const r = run(['touch', '--root', dir, '--pid', String(holder)]);
+    expect(r.status, r.stdout + r.stderr).toBe(0);
+    expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false);
+    expect(fs.readFileSync(path.join(leases, String(holder)), 'utf8')).toMatch(/^(linux|ps) /);
+    fs.mkdirSync(path.join(leases, '.lock'));
+    fs.writeFileSync(path.join(leases, String(holder)), otherStart(fs.readFileSync(path.join(leases, String(holder)), 'utf8').trim()) + '\n');
+    const held = run(['live', '--root', dir]);
+    expect(held.status, held.stdout + held.stderr).toBe(0);
+    expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true);
+  }, 30_000);
+
   test('a lease without a recorded identity (older gstack) counts while fresh and is stale past the 120-minute window', () => {
     const { dir, leases } = checkout();
     const holder = sleeper();