fix(lease): third /ship review pass

- A claim is honored while its claimant lives; only a claim with no start
  identity expires after 2 minutes, so a stalled live taker keeps its lock on
  hosts that record identities (the remaining case is a documented limit).
- A late taker never overwrites an owner already in its directory, and does
  not remove a directory that has one.
- A lock whose owner file exists but cannot be read is never ownerless.
- Zero-padded owner pids count as dead; --help prints the whole usage;
  touch prunes only claims whose claimant is gone or recorded no identity.

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

CHANGELOG.md

@@ -5,7 +5,7 @@
 **Skills start at once on Windows again: the workflow lease lock no longer depends on symlinks (#3330).** `bin/gstack-lease` took its lock with `ln -s "<holder token>" .lock`. In Git for Windows' default symlink mode (`MSYS` unset), `ln -s` cannot link to a target that does not exist, so the lock was never taken. Every skill start then waited out all 300 tries (0.1 s apart, plus process start costs) before `gstack-skill-start` printed anything, and still wrote no lease. One report measured 499 s per skill start. The error also blamed a "live process" with an empty pid.
 
 ### Fixed
-- **`bin/gstack-lease`**: the lock is now a `.lock` directory taken with `mkdir`, which is atomic on every host gstack runs on, with an `owner` file naming the holder's pid and start identity. A taker writes a claim (`.lock.claim.<pid>`) before its `mkdir` and renames it in as the owner after, so a lock left ownerless by a taker killed between the two steps is broken once no claimant younger than 2 minutes lives and the lock is at least 2 s old and unchanged while it was checked (a live taker between its two steps is never mistaken for a crash, and a new lock made meanwhile is never removed). An owner that names no valid pid (0 or not a number) counts as a dead holder, and an empty owner, a stray file at `.lock`, or an ownerless lock whose mtime is a minute or more in the future (clock skew) counts as ownerless instead of wedging every skill start. A lock is removed by renaming it aside first, so nobody sees it half-removed; once renamed it is released even if the renamed copy cannot be deleted, and `touch` prunes leftover copies and claims older than 2 minutes. A waiter no longer writes a claim or runs `mkdir` while the lock is visibly held, and a live holder that never lets go bounds the wait at 300 tries or 30 s, whichever comes first (a non-numeric `GSTACK_LEASE_LOCK_TRIES` falls back to 300 instead of looping forever). Stale-holder semantics are unchanged: a lock is broken only when its holder is dead or its pid now belongs to another process, breakers still serialize on `.lock.break` and reread before removing, and unlock removes only the caller's own lock. A symlink `.lock` left by an older gstack is still read and broken the same way, and it excludes the new lock at the same path. The reverse does not hold: an older gstack's `ln -s` onto the directory lock links inside it, so a process that started before an upgrade of the same checkout can briefly share the lock (documented as a known limit).
+- **`bin/gstack-lease`**: the lock is now a `.lock` directory taken with `mkdir`, which is atomic on every host gstack runs on, with an `owner` file naming the holder's pid and start identity. A taker writes a claim (`.lock.claim.<pid>`) before its `mkdir` and renames it in as the owner after, so a lock left ownerless by a taker killed between the two steps is broken once no claimant lives and the lock is at least 2 s old and unchanged while it was checked (a live taker between its two steps is never mistaken for a crash, and a new lock made meanwhile is never removed). A claim that records no start identity (Git Bash has none) is abandoned after 2 minutes, so a reused pid cannot keep an orphan forever. An owner that names no valid pid (0, zero-padded or not a number) counts as a dead holder, an owner file that exists but cannot be read keeps the lock, and an empty owner, a stray file at `.lock`, or an ownerless lock whose mtime is a minute or more in the future (clock skew) counts as ownerless instead of wedging every skill start. A lock is removed by renaming it aside first, so nobody sees it half-removed; once renamed it is released even if the renamed copy cannot be deleted, and `touch` prunes leftover copies and claims over 2 minutes old whose claimant is gone or recorded no identity. A late taker never overwrites an owner already in the lock it made. A waiter no longer writes a claim or runs `mkdir` while the lock is visibly held, and a live holder that never lets go bounds the wait at 300 tries or 30 s, whichever comes first (a non-numeric `GSTACK_LEASE_LOCK_TRIES` falls back to 300 instead of looping forever). Stale-holder semantics are unchanged: a lock is broken only when its holder is dead or its pid now belongs to another process, breakers still serialize on `.lock.break` and reread before removing, and unlock removes only the caller's own lock. A symlink `.lock` left by an older gstack is still read and broken the same way, and it excludes the new lock at the same path. The reverse does not hold: an older gstack's `ln -s` onto the directory lock links inside it, so a process that started before an upgrade of the same checkout can briefly share the lock (documented as a known limit, with a taker stalled over 2 minutes mid-take on a host without start identities).
 - A dead holder's lock that cannot be renamed aside (a Windows file in use) is removed in place under `.lock.break`, and when that fails too the try is counted, so `touch` ends with exit 3 instead of spinning without sleeping. The message no longer calls a dead holder live.
 - The 120-minute prune in `touch` no longer reaches into the lock: it removed the `owner` file of a holder that resumed past the window, which then left its own lock behind.
 - When the lock cannot be taken although nobody holds it, `touch` now fails after ten quick retries with `gstack-lease: cannot take the lease directory lock <path>` instead of waiting out every try and naming an empty pid. `gstack-skill-start` stays fail-open, as before.
@@ -14,7 +14,7 @@
 ### Tests
 - `test/gstack-lease-git-bash.test.ts` (new, in the curated windows-free-tests lane, launched through `lib/script-invocation.ts`, so Git Bash on `windows-latest`): `touch` with the default 300 tries takes the lock on its first try (no sleep between tries), writes its lease and leaves no lock. On `windows-latest` the old bin fails this case; a host whose `ln -s` fails like Git Bash's still takes the lock (with a control that the shim really fails `ln -s`); a lock held by a live Git Bash process is kept and named, and a dead holder's lock is broken. The last two fail against the old bin.
 - `test/gstack-lease.test.ts`: the dead-holder, live-holder and dead-breaker cases run against both the directory lock and the legacy symlink lock. The race cases inject at `mkdir` instead of `ln`. New cases: a lock held by a running gstack-lease excludes a second taker, which names it; 16 parallel touches all land and leave nothing behind; an ownerless lock is kept while a claimant lives and broken once none does; a lock that cannot be taken while nobody holds it fails without sleeping between tries. Seven of these fail against the old bin.
-- From /ship's coverage audit and review: the 16 parallel touches now also prove no two are inside the lock at once (a mutant whose `mkdir` always succeeds fails it); the exclusion case holds the lock until the test releases it instead of for a 3 s sleep; a young ownerless lock is kept and malformed locks (empty owner, pid 0, a non-numeric owner, a stray file, an mtime an hour ahead) are broken; a claim older than 2 minutes neither keeps an ownerless lock nor survives `touch`; a live holder bounds `touch` by the 30 s deadline with the default and with a non-numeric tries value, without one `mkdir` on the held lock; the prune keeps a resumed holder's lock; a renamed-aside lock that cannot be deleted never takes a new taker's lock with it; a dead holder's lock that cannot be renamed aside ends `touch` instead of spinning. Each case that pins a fix fails against the bin before that fix. Also covered: a live or ownerless `.lock.break`, a failed owner rename, a lock released between a failed take and its check, and unlock's fallback. The Git Bash case now also checks that the live holder still owns the lock after a blocked `touch`.
+- From /ship's coverage audit and review: the 16 parallel touches now also prove no two are inside the lock at once (a mutant whose `mkdir` always succeeds fails it); the exclusion case holds the lock until the test releases it instead of for a 3 s sleep; a young ownerless lock is kept and malformed locks (empty owner, pid 0 or 00, a non-numeric owner, a stray file, an mtime an hour ahead) are broken; an identity-less claim older than 2 minutes neither keeps an ownerless lock nor survives `touch` while an old claim with a live identity is honored; an unreadable owner file keeps the lock; `--help` prints the whole usage again; leftover renamed-aside copies are pruned; a live holder bounds `touch` by the 30 s deadline with the default and with a non-numeric tries value, without one `mkdir` on the held lock; the prune keeps a resumed holder's lock; a renamed-aside lock that cannot be deleted never takes a new taker's lock with it; a dead holder's lock that cannot be renamed aside ends `touch` instead of spinning. Each case that pins a fix fails against the bin before that fix. Also covered: a live or ownerless `.lock.break`, a failed owner rename, a lock released between a failed take and its check, and unlock's fallback. The Git Bash case now also checks that the live holder still owns the lock after a blocked `touch`.
 
 ### Gate integrity
 - No threshold, budget, timeout, trial count, case kind or assertion was weakened. Three existing race tests moved their fault injection from `ln` to `mkdir`, because the lock no longer calls `ln`. Their assertions are unchanged. Under /ship review, the exclusion case's wait for the holder's owner file rose from 2 s to 10 s and its fixed 3 s hold became a release file (no assertion changed), the ownerless-lock cases backdate their planted lock by a minute because an ownerless lock is now broken only once 2 s old (their assertions are unchanged; the new young-lock case pins the guard), and the exclusion and parallel cases got an explicit 30 s test timeout instead of Bun's 5 s default.

bin/gstack-lease

@@ -32,7 +32,7 @@
 # runtime root's .source-path, else ../). A root that is not a git checkout
 # exits 2 and writes nothing. GSTACK_LEASE_DIR overrides the lease directory
 # (hermetic tests; a dev checkout's own tests must not lease it). Bash 3.2
-# plus git, ps, find, mkdir, mv, readlink.
+# plus git, ps, find, stat, date, head, mkdir, mv, readlink.
 set -u
 BIN="$(cd "$(dirname "$0")" && pwd -P)"
 # shellcheck source=bin/gstack-install-verdict.sh
@@ -54,7 +54,7 @@ while [ $# -gt 0 ]; do
 done
 case "$VERB" in
   touch|live|list) ;;
-  -h|--help|'') sed -n '2,24p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;;
+  -h|--help|'') sed -n '2,25p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;;
   *) echo "gstack-lease: unknown verb '$VERB'" >&2; exit 2 ;;
 esac
 [ -n "$ROOT" ] || ROOT=$(gstack_source_root "$BIN")
@@ -112,9 +112,13 @@ state() {
 # broken once no claimant lives and it is at least 2 s old (and unchanged
 # while it was checked), so a taker between its mkdir and its owner rename is
 # never taken for a crash, even when its claim landed after the claims were
-# read; a claim older than 2 minutes is abandoned, since a live taker holds
-# one for milliseconds. An owner token whose pid is 0 or not a number counts
-# as a dead holder. A symlink .lock left by an older
+# read. A claim is honored while its claimant lives; one that records no
+# start identity (so a reused pid cannot be told apart) is abandoned after 2
+# minutes, since a live taker holds one for milliseconds. Known limit: on such
+# a host a taker stalled over 2 minutes between its mkdir and its owner rename
+# can be broken and then publish into its successor's lock. An owner whose
+# pid is 0, zero-padded or not a number counts as a dead holder, and a lock
+# whose owner file exists but cannot be read is never treated as ownerless. A symlink .lock left by an older
 # gstack names its holder the same way and excludes a directory lock at the
 # same path. Known limit: the reverse does not hold, since an older gstack's
 # ln -s onto a directory lock links inside it and succeeds; only a process
@@ -127,33 +131,36 @@ take() {
   if [ -e "$1" ] || [ -L "$1" ]; then return 1; fi
   { printf '%s\n' "$TOKEN" > "$1.claim.$$"; } 2>/dev/null || { rm -f "$1.claim.$$"; return 1; }
   if mkdir "$1" 2>/dev/null; then
-    mv -f "$1.claim.$$" "$1/owner" && return 0
-    rm -rf "$1"
+    [ ! -e "$1/owner" ] && mv -f "$1.claim.$$" "$1/owner" && return 0
+    [ -e "$1/owner" ] || rm -rf "$1"
   fi
   rm -f "$1.claim.$$"
   return 1
 }
 # owner_of PATH — the token the lock PATH names, empty when it names none.
 owner_of() { if [ -L "$1" ]; then readlink "$1" 2>/dev/null; else head -1 "$1/owner" 2>/dev/null; fi; }
-# orphaned PATH — PATH is a lock (not a legacy symlink) that names no owner,
-# has no live claimant younger than 2 minutes, is at least 2 s old and kept
+mtime() { stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null; }
+# orphaned PATH — PATH is a lock (not a legacy symlink) whose owner file is
+# missing or readable and empty, has no live claimant, is at least 2 s old and kept
 # its mtime throughout. The clock is read before the first mtime, so a pause
 # only makes the lock look younger, and a lock made since (a new generation)
 # has a newer mtime. Claims are read before the owner, so a taker that renames
 # its claim in between is seen as the owner. An empty claim is a taker that
 # has not reached its mkdir, so it never made this lock. An mtime a minute or
 # more in the future (clock skew) counts as old.
-mtime() { stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null; }
 orphaned() {
   _oclock=$(date +%s)
   [ -e "$1" ] && [ ! -L "$1" ] || return 1
   _om=$(mtime "$1") || return 1
   for _c in "$1".claim.*; do
-    [ -n "$(find "$_c" -mmin -2 2>/dev/null)" ] || continue
     _t=$(head -1 "$_c" 2>/dev/null)
+    abandoned "$_c" "$_t" && continue
     [ -n "$_t" ] && ! holder_gone "$_t" && return 1
   done
-  [ -z "$(owner_of "$1")" ] || return 1
+  if [ -d "$1" ] && [ -e "$1/owner" ]; then
+    _o=$(head -1 "$1/owner" 2>/dev/null) || return 1
+    [ -z "$_o" ] || return 1
+  fi
   [ "$(mtime "$1")" = "$_om" ] || return 1
   _oage=$((_oclock - _om))
   [ "$_oage" -ge 2 ] || [ "$_oage" -le -60 ]
@@ -189,8 +196,11 @@ lock() {
     sleep 0.1
   done
 }
+# abandoned CLAIM TOKEN — the claim records no start identity and is over 2
+# minutes old.
+abandoned() { [ -z "${2#* }" ] && [ -z "$(find "$1" -mmin -2 2>/dev/null)" ]; }
 holder_gone() {
-  case "${1%% *}" in ''|*[!0-9]*|0) return 0 ;; esac
+  case "${1%% *}" in ''|*[!0-9]*|0*) return 0 ;; esac
   case "$(state "${1%% *}" "${1#* }" 0)" in dead|stale) return 0 ;; esac; return 1
 }
 # break_lock OWNER — remove the lock only if it still names OWNER (empty: is
@@ -249,7 +259,11 @@ case "$VERB" in
     _tmp="$DIR/.$PID.$$"
     identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; unlock; exit 2; }
     find "$DIR" -maxdepth 1 -mmin +120 -type f ! -name '.lock*' -exec rm -f {} + 2>/dev/null
-    find "$DIR" -maxdepth 1 -mmin +2 -type f -name '.lock*.claim.*' -exec rm -f {} + 2>/dev/null
+    for _c in "$DIR"/.lock*.claim.*; do
+      [ -f "$_c" ] || continue
+      _t=$(head -1 "$_c" 2>/dev/null)
+      [ -n "$(find "$_c" -mmin +2 2>/dev/null)" ] && { abandoned "$_c" "$_t" || holder_gone "$_t"; } && rm -f "$_c"
+    done
     rm -rf "$DIR"/.lock*.gone.* 2>/dev/null
     unlock
     exit 0 ;;

docs/capy-install.md

@@ -126,17 +126,21 @@ deleted and a live lease never disappears from a concurrent scan. The lock is a
 directory because `mkdir` is atomic on every host, while Git Bash on Windows
 cannot `ln -s` to a target that does not exist (#3330). A taker publishes a
 claim before its `mkdir` and renames it in as the owner after, so an ownerless
-lock is broken only once no claimant younger than 2 minutes lives and the
-lock is at least 2 s old and unchanged while checked, which keeps a taker
-between its two steps safe; an owner that names no valid pid counts as a dead
-holder. A live holder that never lets go bounds the wait at 300 tries or 30 s. The lock is broken only when its holder is dead or
+lock is broken only once no claimant lives and the lock is at least 2 s old
+and unchanged while checked, which keeps a taker between its two steps safe.
+A claim that records no start identity is abandoned after 2 minutes, so a
+reused pid cannot keep an orphan forever; an owner that names no valid pid
+counts as a dead holder, and an owner file that cannot be read keeps the
+lock. A live holder that never lets go bounds the wait at 300 tries or 30 s. The lock is broken only when its holder is dead or
 its pid was reused, never by age, and breakers serialize on a second lock and
 reread the lock before removing it. A symlink `.lock` left by an older gstack
-is honored the same way. Two known limits remain. An older gstack's `ln -s`
+is honored the same way. Three known limits remain. An older gstack's `ln -s`
 onto a directory lock links inside it and succeeds, so a process that started
 before an upgrade of this checkout can share the lock with a newer one. And a
 breaker killed while holding the second lock, followed by two live breakers
-clearing it at once, can let a reclaim delete a renewal. While a live process
+clearing it at once, can let a reclaim delete a renewal. On a host without start
+identities, a taker stalled over 2 minutes between its `mkdir` and its owner
+rename can be broken and then publish into its successor's lock. While a live process
 holds the lock, or it cannot be taken at all, `touch` exits 3 without writing
 (`gstack-skill-start` reports that the session holds no lease) and a reclaim
 keeps the lease. `bin/gstack-lease list` shows `alive|dead|stale`, and

test/gstack-lease.test.ts

@@ -90,6 +90,8 @@ describe('gstack-lease', () => {
     expect(fs.statSync(LEASE).mode & 0o111).not.toBe(0);
     const r = spawnSync('bash', ['-n', LEASE], { encoding: 'utf8', timeout: 30_000 });
     expect(r.status, r.stderr).toBe(0);
+    const help = spawnSync('bash', [LEASE, '--help'], { encoding: 'utf8', timeout: 30_000 });
+    expect(help.stdout).toContain('print `<pid> <age>m alive|dead|stale` for every lease');
   });
 
   test('touch writes <git dir>/gstack-leases/<pid> for --pid and prunes leases older than 120 minutes', () => {
@@ -365,6 +367,7 @@ describe('gstack-lease', () => {
     const shapes: Array<[string, () => void]> = [
       ['an empty owner file', () => plantLock(lockPath, '')],
       ['owner pid 0', () => plantLock(lockPath, '0 ')],
+      ['a zero-padded owner pid', () => plantLock(lockPath, '00 ')],
       ['a non-numeric owner', () => plantLock(lockPath, 'garbage x')],
       ['an ownerless lock an hour ahead (clock skew)', () => fs.mkdirSync(lockPath)],
       ['a stray file', () => { fs.rmSync(lockPath, { recursive: true, force: true }); fs.writeFileSync(lockPath, ''); }],
@@ -384,12 +387,12 @@ describe('gstack-lease', () => {
     }
   });
 
-  // Value: protects=a claim left by a killed taker neither blocks recovery of an ownerless lock (even when its pid is alive again) nor lingers:
-  //   a claim older than 2 minutes is abandoned, and touch prunes it;
-  //   fails_when=orphaned() honors claims of any age, or touch's prune skips claim files;
+  // Value: protects=a claim without a start identity left by a killed taker neither blocks recovery of an ownerless lock (even when its pid is
+  //   alive again) nor lingers once 2 minutes old, while a claim whose recorded identity is still live is honored however old;
+  //   fails_when=orphaned() honors identity-less claims of any age, expires live identified claims, or touch's prune skips claim files;
   //   why_new=the ownerless cases plant fresh claims only; nothing ages one or checks the prune for claims;
   //   seam=none; no_claim=real pid reuse (a live sleeper stands in for the reused pid)
-  test('a claim older than 2 minutes is abandoned: it neither keeps an ownerless lock nor survives touch', () => {
+  test('an identity-less claim older than 2 minutes is abandoned and pruned; an old claim with a live identity is honored', () => {
     const { dir, leases } = checkout();
     const [holder, reused] = [sleeper(), sleeper()];
     const lockPath = path.join(leases, '.lock');
@@ -402,6 +405,37 @@ describe('gstack-lease', () => {
     const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' });
     expect(r.status, r.stdout + r.stderr).toBe(0);
     expect(fs.readdirSync(leases)).toEqual([String(holder)]);
+
+    // A claim that records its claimant's start identity is honored however old, so a stalled live taker keeps its lock.
+    expect(run(['touch', '--root', dir, '--pid', String(reused)]).status).toBe(0);
+    const identity = fs.readFileSync(path.join(leases, String(reused)), 'utf8').trim();
+    expect(identity, 'this host records start identities').not.toBe('');
+    fs.rmSync(path.join(leases, String(reused)));
+    fs.mkdirSync(lockPath);
+    age(lockPath);
+    fs.writeFileSync(claim, `${reused} ${identity}\n`);
+    fs.utimesSync(claim, fiveMinutesAgo, fiveMinutesAgo);
+    const kept = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' });
+    expect(kept.status, kept.stdout + kept.stderr).toBe(3);
+    expect(fs.existsSync(lockPath)).toBe(true);
+    expect(fs.existsSync(claim)).toBe(true);
+  });
+
+  // Value: protects=a live holder's lock is never broken because its owner file could not be read (a Windows sharing violation, a permission error);
+  //   fails_when=orphaned() treats an unreadable owner (empty owner_of output) as no owner;
+  //   why_new=every ownerless case removes or empties the owner file; none makes a present owner unreadable;
+  //   seam=none; no_claim=a real sharing violation (a head shim fails reads of the owner file)
+  test('a lock whose owner file cannot be read is kept, not taken for ownerless', () => {
+    const { dir, leases } = checkout();
+    const [holder, owner] = [sleeper(), sleeper()];
+    const lockPath = path.join(leases, '.lock');
+    plantLock(lockPath, `${owner} `);
+    age(lockPath);
+    const shim = bashEnv(dir, 'head() { case "$*" in *"/.lock/owner") return 1 ;; esac; command head "$@"; }\n');
+    const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3', BASH_ENV: shim });
+    expect(r.status, r.stdout + r.stderr).toBe(3);
+    expect(ownerOf(lockPath)).toBe(String(owner));
+    expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false);
   });
 
   // Value: protects=a live holder that never lets go bounds touch at 30 s even with the default 300 tries, a non-numeric
@@ -439,6 +473,10 @@ describe('gstack-lease', () => {
     const { dir, leases } = checkout();
     const holder = sleeper();
     const shim = bashEnv(dir, 'find() { command touch -d "3 hours ago" "$1/.lock/owner" 2>/dev/null || command touch -t 197001010000 "$1/.lock/owner"; command find "$@"; }\n');
+    // Renamed-aside copies a failed delete left behind are pruned too.
+    fs.mkdirSync(path.join(leases, '.lock.gone.99999'), { recursive: true });
+    fs.writeFileSync(path.join(leases, '.lock.gone.99999', 'owner'), '99999 \n');
+    fs.mkdirSync(path.join(leases, '.lock.break.gone.99998'));
     const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim });
     expect(r.status, r.stdout + r.stderr).toBe(0);
     expect(fs.readdirSync(leases)).toEqual([String(holder)]);