diff --git a/CHANGELOG.md b/CHANGELOG.md index 8353ed224..d01ec79b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## [1.91.228.0] - 2026-10-26 + +**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 "" .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.`) 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 ` instead of waiting out every try and naming an empty pid. `gstack-skill-start` stays fail-open, as before. +- `docs/capy-install.md` describes the directory lock. + +### 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 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. + ## [1.91.227.0] - 2026-10-26 **Windows: gstack's record bins stop losing review, question, learning and timeline records when Git Bash path conversion is off (#3415).** Since GSTA-56 every shell bin started Bun with `--config=/dev/null`. Native `bun.exe` cannot open `/dev/null`: it exits 1 with `EINVAL` on Bun 1.3.10, 1.4.2 and 1.4.4 (measured on windows-latest). Git Bash hides this by rewriting the argument to `nul`, but only while MSYS path conversion is on. With it off (`MSYS_NO_PATHCONV` set to any value, even empty; `MSYS2_ARG_CONV_EXCL=*`; or Cygwin), `gstack-review-log`, `gstack-question-log` and `gstack-learnings-log` exited 1 with nothing written, and `gstack-timeline-log` exited 0 with nothing written. diff --git a/VERSION b/VERSION index 0831dfe12..7d734b27e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.227.0 +1.91.228.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 073efdb52..b4e5fdadb 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.227.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.228.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-lease b/bin/gstack-lease index ded79313e..2073af92d 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -14,7 +14,8 @@ # (bin/gstack-harness-pid.sh, the same derivation the session marker # uses), or behind --from (a caller whose own parent is short-lived # passes its parent pid); leases older than 120 min are pruned. Exits 3 -# and writes nothing when a live process holds the directory lock +# and writes nothing when a live process holds the directory lock or +# the lock cannot be taken # gstack-lease live [--root ] [--exclude-ancestry-of ] # print ` m ` per lease held by a live process that # is neither the given pid nor one of its ancestors (the caller's own @@ -31,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, ln, 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 @@ -53,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") @@ -99,43 +100,139 @@ state() { } # 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. The lock is a symlink whose target -# names its holder (` `): ln -s creates it and its owner -# in one atomic step. It is broken only when that holder is dead or its pid -# now belongs to another process, never by age, and unlock removes only the -# caller's own lock. A live holder makes lock() give up after -# GSTACK_LEASE_LOCK_TRIES tries 0.1 s apart (default 300). +# lease never disappears while it is live. The lock is a directory, .lock, +# whose owner file names its holder (` `); mkdir is +# atomic everywhere gstack runs, Git Bash on Windows included, where ln -s to a +# target that does not exist always fails (#3330). A taker publishes its token +# as a claim (.lock.claim.) before its mkdir and renames the claim into +# the lock as its owner after, so a lock without an owner is either being +# taken by a live claimant or orphaned by a dead one. A lock is broken only +# when its holder is dead or its pid now belongs to another process, never by +# age, and unlock removes only the caller's own lock. An ownerless lock is +# 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 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 +# that started before an upgrade of this checkout can run that code. A live holder +# makes lock() give up after GSTACK_LEASE_LOCK_TRIES tries 0.1 s apart +# (default 300) or 30 s, whichever comes first. LOCK="$DIR/.lock" TOKEN="" +# take PATH — create the lock PATH owned by TOKEN; 1 when it exists. +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 + [ ! -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; } +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. +orphaned() { + _oclock=$(date +%s) + [ -e "$1" ] && [ ! -L "$1" ] || return 1 + _om=$(mtime "$1") || return 1 + for _c in "$1".claim.*; do + _t=$(head -1 "$_c" 2>/dev/null) + abandoned "$_c" "$_t" && continue + [ -n "$_t" ] && ! holder_gone "$_t" && return 1 + done + 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 ] +} +# drop PATH — remove a lock in one step: a directory is renamed aside first, +# so nobody sees it ownerless on the way out. Once renamed the lock is +# released, whether or not the renamed copy can be deleted (touch prunes it). +drop() { + if [ -L "$1" ]; then rm -f "$1"; return; fi + mv "$1" "$1.gone.$$" 2>/dev/null || return 1 + rm -rf "$1.gone.$$" 2>/dev/null + return 0 +} lock() { [ -n "$TOKEN" ] || TOKEN="$$ $(identity $$)" - _tries=0 - until ln -s "$TOKEN" "$LOCK" 2>/dev/null; do - _tries=$((_tries + 1)) - LOCK_OWNER=$(readlink "$LOCK" 2>/dev/null) - [ -n "$LOCK_OWNER" ] && holder_gone "$LOCK_OWNER" && break_lock "$LOCK_OWNER" && continue - [ "$_tries" -ge "${GSTACK_LEASE_LOCK_TRIES:-300}" ] && return 1 + _tries=0 _gone=0 _max=${GSTACK_LEASE_LOCK_TRIES:-300} _start=$SECONDS + case "$_max" in ''|*[!0-9]*) _max=300 ;; esac + until take "$LOCK"; do + LOCK_OWNER=$(owner_of "$LOCK") + if [ ! -e "$LOCK" ] && [ ! -L "$LOCK" ]; then + # Released since take, or take itself cannot work here: never wait out + # the tries for a lock nobody holds. + _gone=$((_gone + 1)) + [ "$_gone" -ge 10 ] && return 1 + continue + fi + _gone=0 _tries=$((_tries + 1)) + if [ -n "$LOCK_OWNER" ]; then holder_gone "$LOCK_OWNER" && break_lock "$LOCK_OWNER" && continue + elif orphaned "$LOCK"; then break_lock "" && continue + fi + [ "$_tries" -ge "$_max" ] && return 1 + [ $((SECONDS - _start)) -ge 30 ] && return 1 sleep 0.1 done } -holder_gone() { 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. Breakers -# take a second symlink, .lock.break, and reread the lock under it, so a lock -# taken since the verdict is never removed; a .lock.break whose holder is gone -# is cleared. Returns 1 when another breaker holds .lock.break. Known limit: -# that clear is not atomic, so two gstack-lease processes killed in a row (one -# holding .lock, the next inside this function) can let a reclaim delete a -# renewal; one crash is always safe. +# 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 "$(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 +# still orphaned). Breakers take a second lock, .lock.break, and reread the +# lock under it, so a lock taken since the verdict is never removed; a +# .lock.break whose holder is gone is cleared. A lock that cannot be renamed +# aside is removed in place (other breakers wait on .lock.break meanwhile). +# Returns 1 when another breaker holds .lock.break or the lock could not be +# removed, so lock() counts the try instead of spinning. Known limit: that +# clear is check-then-remove, so a breaker killed while holding .lock.break, +# followed by two live breakers clearing it at once, can let both break and a +# reclaim delete a renewal. break_lock() { - if ! ln -s "$TOKEN" "$LOCK.break" 2>/dev/null; then - _b=$(readlink "$LOCK.break" 2>/dev/null) - [ -n "$_b" ] && holder_gone "$_b" && [ "$(readlink "$LOCK.break" 2>/dev/null)" = "$_b" ] && rm -f "$LOCK.break" + if ! take "$LOCK.break"; then + _b=$(owner_of "$LOCK.break") + if [ -n "$_b" ]; then + holder_gone "$_b" && [ "$(owner_of "$LOCK.break")" = "$_b" ] && drop "$LOCK.break" + else + orphaned "$LOCK.break" && drop "$LOCK.break" + fi return 1 fi - [ "$(readlink "$LOCK" 2>/dev/null)" = "$1" ] && rm -f "$LOCK" - rm -f "$LOCK.break" - return 0 + _kept=0 + if [ -n "$1" ]; then + [ "$(owner_of "$LOCK")" = "$1" ] && { drop "$LOCK" || rm -rf "$LOCK" || _kept=1; } + else + orphaned "$LOCK" && { drop "$LOCK" || rm -rf "$LOCK" || _kept=1; } + fi + drop "$LOCK.break" + return $_kept } -unlock() { [ "$(readlink "$LOCK" 2>/dev/null)" = "$TOKEN" ] && rm -f "$LOCK"; return 0; } +unlock() { [ "$(owner_of "$LOCK")" = "$TOKEN" ] && { drop "$LOCK" || rm -rf "$LOCK"; }; return 0; } # 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() { @@ -153,10 +250,21 @@ case "$VERB" in touch) [ -n "$PID" ] || PID=$(gstack_harness_pid "${FROM:-$PPID}") mkdir -p "$DIR" || exit 2 - lock || { echo "gstack-lease: the lease directory is locked by live process ${LOCK_OWNER%% *}; lease $PID not renewed" >&2; exit 3; } + lock || { + if [ -n "${LOCK_OWNER:-}" ] && ! holder_gone "$LOCK_OWNER"; then echo "gstack-lease: the lease directory is locked by live process ${LOCK_OWNER%% *}; lease $PID not renewed" >&2 + else echo "gstack-lease: cannot take the lease directory lock $LOCK; lease $PID not renewed" >&2 + fi + exit 3 + } _tmp="$DIR/.$PID.$$" identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; unlock; exit 2; } - find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null + find "$DIR" -maxdepth 1 -mmin +120 -type f ! -name '.lock*' -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 ;; list) diff --git a/docs/capy-install.md b/docs/capy-install.md index c2d0ce97b..dd272cb18 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -120,15 +120,30 @@ 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. `touch` (publish by rename, prune) and reclaim (reread, then delete only the record judged -stale) run under one lock in the lease directory, a symlink naming its holder's -pid and start identity, so a renewal is never deleted and a live lease never -disappears from a concurrent scan. The lock is broken only when its holder is -dead or its pid was reused, never by age, and breakers serialize on a second -symlink and reread the lock before removing it. One killed gstack-lease process -is always safe; two killed in a row, one holding the lock and the next while -breaking it, can still let a reclaim delete a renewal. While a live process holds it, -`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 +stale) run under one lock in the lease directory, a `.lock` directory whose +`owner` file names its holder's pid and start identity, so a renewal is never +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 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. A lock that names a holder is broken only when that 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. 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. 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 `GSTACK_UPGRADE_FORCE=1` overrides. ## What the installer does diff --git a/package.json b/package.json index 316846110..f66c1ffae 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.227", + "version": "1.91.228", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/gstack-lease-git-bash.test.ts b/test/gstack-lease-git-bash.test.ts new file mode 100644 index 000000000..61cc68c18 --- /dev/null +++ b/test/gstack-lease-git-bash.test.ts @@ -0,0 +1,94 @@ +/** + * #3330: gstack-lease took its lock with `ln -s .lock`. Git + * for Windows' default symlink mode (MSYS unset) cannot link to a target that + * does not exist, so the lock was never taken and every skill start waited out + * 300 tries (one report: 499 s) and still wrote no lease. The lock is now a + * directory (mkdir) with an owner file. test/gstack-lease.test.ts pins the full + * contract but is POSIX-bound (mode bits, sleep children), so this file is the + * one the curated windows-free-tests lane runs: it launches the bin through + * lib/script-invocation.ts, which is Git Bash on windows-latest. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { BIN_DIR, runBashSync, runScriptSync } from '../lib/script-invocation'; + +const LEASE = path.join(BIN_DIR, 'gstack-lease'); +const fwd = (p: string) => p.replace(/\\/g, '/'); +const dirs: string[] = []; +afterEach(() => { + for (const d of dirs.splice(0)) fs.rmSync(d, { recursive: true, force: true }); +}); + +// No inherited lease dir from the test preload, and Git Bash's default symlink mode (no winsymlinks). +const { GSTACK_LEASE_DIR: _preload, MSYS: _msys, CYGWIN: _cygwin, GSTACK_LEASE_LOCK_TRIES: _tries, ...baseEnv } = process.env as Record; + +function checkout() { + const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-lease-gb-'))); + dirs.push(dir); + const r = spawnSync('git', ['init', '-q', dir], { encoding: 'utf8', timeout: 30_000 }); + if (r.status !== 0) throw new Error(r.stderr); + return { dir, leases: path.join(dir, '.git', 'gstack-leases') }; +} + +/** A BASH_ENV file in which `ln -s` behaves as Git Bash's default: linking to a missing target fails. */ +function gitBashLn(dir: string): string { + const file = path.join(dir, 'git-bash-ln.sh'); + fs.writeFileSync(file, 'ln() { if [ "$1" = -s ] && [ ! -e "$2" ]; then echo "ln: failed to create symbolic link \'$3\': No such file or directory" >&2; return 1; fi; command ln "$@"; }\n'); + return file; +} + +/** A BASH_ENV file that logs every `sleep` (one per failed lock try) to $WAITS. */ +function sleepLog(dir: string): string { + const file = path.join(dir, 'sleep-log.sh'); + fs.writeFileSync(file, 'sleep() { echo "$*" >> "$WAITS"; command sleep "$@"; }\n'); + return file; +} + +describe('gstack-lease under Git Bash semantics (#3330)', () => { + test('touch takes the lock on its first try with the default tries and writes the lease, leaving no lock behind', () => { + const { dir, leases } = checkout(); + const waits = path.join(dir, 'waits.log'); + const r = runScriptSync(LEASE, ['touch', '--root', fwd(dir), '--pid', '4242'], { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(sleepLog(dir)), WAITS: fwd(waits) }, timeout: 120_000 }); + expect(r.status, `${r.error ?? ''}${r.stdout}${r.stderr}`).toBe(0); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(fs.readdirSync(leases)).toEqual(['4242']); + }); + + test('a host whose ln -s cannot link to a missing target still takes the lock (negative control: the shim really fails ln -s)', () => { + const { dir, leases } = checkout(); + const shim = gitBashLn(dir); + const probe = runBashSync('ln -s "1 2" "$PROBE"', { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(shim), PROBE: fwd(path.join(dir, 'probe')) }, timeout: 30_000 }); + expect(probe.status, `${probe.error ?? ''}${probe.stderr}`).not.toBe(0); + expect(probe.stderr).toContain('failed to create symbolic link'); + const r = runScriptSync(LEASE, ['touch', '--root', fwd(dir), '--pid', '4243'], { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(shim), GSTACK_LEASE_LOCK_TRIES: '3' }, timeout: 120_000 }); + expect(r.status, `${r.error ?? ''}${r.stdout}${r.stderr}`).toBe(0); + expect(fs.readdirSync(leases)).toEqual(['4243']); + }); + + test('a lock held by a live Git Bash process is kept (touch exits 3 naming it); a dead holder\'s lock is broken', () => { + const { dir, leases } = checkout(); + const script = [ + 'set -u', + 'mkdir -p "$LEASES/.lock" && printf "%s \\n" "$$" > "$LEASES/.lock/owner"', + 'GSTACK_LEASE_LOCK_TRIES=2 bash "$LEASE" touch --root "$ROOT" --pid 4244; echo "held=$? holder=$$ owner=$(head -1 "$LEASES/.lock/owner")"', + '( exit 0 ) & dead=$!; wait "$dead"', + 'printf "%s \\n" "$dead" > "$LEASES/.lock/owner"', + 'GSTACK_LEASE_LOCK_TRIES=2 bash "$LEASE" touch --root "$ROOT" --pid 4245; echo "dead=$?"', + ].join('\n'); + const r = runBashSync(script, { + encoding: 'utf8', timeout: 120_000, + env: { ...baseEnv, LEASE: fwd(LEASE), ROOT: fwd(dir), LEASES: fwd(leases) }, + }); + const out = `${r.error ?? ''}${r.stdout}${r.stderr}`; + expect(r.status, out).toBe(0); + const holder = /held=(\d+) holder=(\d+) owner=(\d+)/.exec(r.stdout); + expect(holder?.[1], out).toBe('3'); + expect(holder?.[3], out).toBe(holder?.[2]); + expect(r.stderr).toContain(`locked by live process ${holder?.[2]}`); + expect(r.stdout, out).toContain('dead=0'); + expect(fs.readdirSync(leases).sort()).toEqual(['4245']); + }); +}); diff --git a/test/gstack-lease.test.ts b/test/gstack-lease.test.ts index f2b6585d2..05cb4945d 100644 --- a/test/gstack-lease.test.ts +++ b/test/gstack-lease.test.ts @@ -59,6 +59,24 @@ function otherStart(recorded: string): string { return recorded.startsWith('linux ') ? recorded.replace(/\d+$/, (n) => String(Number(n) + 1)) : 'ps Thu Jan 1 00:00:00 1970'; } +/** A directory lock as gstack-lease takes it: /owner holds the holder's token. */ +function plantLock(lockPath: string, token: string) { + fs.mkdirSync(lockPath, { recursive: true }); + fs.writeFileSync(path.join(lockPath, 'owner'), token + '\n'); +} +const ownerOf = (lockPath: string) => fs.readFileSync(path.join(lockPath, 'owner'), 'utf8').trim(); +/** Backdate a lock a minute: an ownerless lock is broken only once it is 2 s old. */ +function age(lockPath: string) { + const minuteAgo = new Date(Date.now() - 60_000); + fs.utimesSync(lockPath, minuteAgo, minuteAgo); +} + +/** Each lock layout a holder can leave: the directory lock, and the symlink lock an older gstack took. */ +const LAYOUTS: Array<[string, (lockPath: string, token: string) => void]> = [ + ['directory lock', plantLock], + ['legacy symlink lock', (lockPath, token) => fs.symlinkSync(token, lockPath)], +]; + /** A BASH_ENV file whose functions shadow commands inside the lease script (fault injection). */ function bashEnv(dir: string, body: string): string { const file = path.join(dir, 'bash-env.sh'); @@ -72,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 ` m alive|dead|stale` for every lease'); }); test('touch writes /gstack-leases/ for --pid and prunes leases older than 120 minutes', () => { @@ -160,7 +180,7 @@ describe('gstack-lease', () => { const renewed = path.join(dir, 'renewed'); fs.writeFileSync(renewed, fresh); // The holder renews just before reclaim takes the lock, as if touch ran after the stale verdict. - const racing = bashEnv(dir, `ln() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command ln "$@"; }\n`); + 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,23 +188,24 @@ describe('gstack-lease', () => { expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]); }); - test('a lock whose holder died, or whose pid now belongs to another process, is broken at once', () => { + test.each(LAYOUTS)('a %s whose holder died, or whose pid now belongs to another process, is broken at once', (_layout, plant) => { const { dir, leases } = checkout(); const holder = sleeper(); fs.mkdirSync(leases, { recursive: true }); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock')); + plant(path.join(leases, '.lock'), `${deadPid()} `); 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.existsSync(path.join(leases, '.lock'))).toBe(false); const identity = fs.readFileSync(path.join(leases, String(holder)), 'utf8').trim(); expect(identity).toMatch(/^(linux|ps) /); - fs.symlinkSync(`${holder} ${otherStart(identity)}`, path.join(leases, '.lock')); + plant(path.join(leases, '.lock'), `${holder} ${otherStart(identity)}`); const again = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); expect(again.status, again.stdout + again.stderr).toBe(0); expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false); + expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]); }); - test('a live holder\'s lock is never broken, however old: touch exits 3 without writing and a reclaim keeps the lease', () => { + test.each(LAYOUTS)('a live holder\'s %s is never broken, however old: touch exits 3 without writing and a reclaim keeps the lease', (layout, plant) => { const { dir, leases } = checkout(); const owner = sleeper(); const holder = sleeper(); @@ -192,9 +213,10 @@ describe('gstack-lease', () => { expect(run(['touch', '--root', dir, '--pid', String(holder)]).status).toBe(0); const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; const lockPath = path.join(leases, '.lock'); - fs.symlinkSync(ownerToken, lockPath); + plant(lockPath, ownerToken); const old = new Date(Date.now() - 5 * 60_000); fs.lutimesSync(lockPath, old, old); + if (layout === 'directory lock') fs.utimesSync(path.join(lockPath, 'owner'), old, old); const holderFile = path.join(leases, String(holder)); const stale = otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'; fs.writeFileSync(holderFile, stale); @@ -207,7 +229,7 @@ describe('gstack-lease', () => { expect(l.stderr).not.toContain('reclaimed'); expect(l.stdout).toContain(`${holder} `); expect(fs.existsSync(holderFile)).toBe(true); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + expect(layout === 'directory lock' ? ownerOf(lockPath) : fs.readlinkSync(lockPath)).toBe(ownerToken); }); test('two breakers of one dead lock: the second rereads under .lock.break and leaves the lock the first one took', () => { @@ -217,21 +239,21 @@ describe('gstack-lease', () => { expect(run(['touch', '--root', dir, '--pid', String(owner)]).status).toBe(0); const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; const lockPath = path.join(leases, '.lock'); - fs.symlinkSync(`${deadPid()} `, lockPath); + plantLock(lockPath, `${deadPid()} `); // Between this breaker's dead verdict and its .lock.break, another breaker removes the dead lock and takes it. - const first = bashEnv(dir, `ln() { case "$*" in *"/.lock.break") command rm -f "${lockPath}"; command ln -s "${ownerToken}" "${lockPath}" ;; esac; command ln "$@"; }\n`); + const first = bashEnv(dir, `mkdir() { case "$*" in *"/.lock.break") command rm -rf "${lockPath}"; command mkdir "${lockPath}"; printf '%s\\n' "${ownerToken}" > "${lockPath}/owner" ;; esac; command mkdir "$@"; }\n`); const t = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: first, GSTACK_LEASE_LOCK_TRIES: '3' }); expect(t.status, t.stdout + t.stderr).toBe(3); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); - expect(fs.existsSync(path.join(leases, '.lock.break'))).toBe(false); + expect(ownerOf(lockPath)).toBe(ownerToken); + expect(fs.readdirSync(leases).filter((n) => n.startsWith('.')).sort()).toEqual(['.lock']); }); - test('a .lock.break left by a dead breaker is cleared and the dead lock is still broken', () => { + test.each(LAYOUTS)('a .lock.break (%s) left by a dead breaker is cleared and the dead lock is still broken', (_layout, plant) => { const { dir, leases } = checkout(); const holder = sleeper(); fs.mkdirSync(leases, { recursive: true }); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock')); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock.break')); + plant(path.join(leases, '.lock'), `${deadPid()} `); + plant(path.join(leases, '.lock.break'), `${deadPid()} `); const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '5' }); expect(r.status, r.stdout + r.stderr).toBe(0); expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); @@ -249,12 +271,355 @@ describe('gstack-lease', () => { fs.writeFileSync(holderFile, otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'); const lockPath = path.join(leases, '.lock'); // While reclaim deletes the stale lease under its lock, the lock changes hands. - const swap = bashEnv(dir, `rm() { if [ "$*" = "-f ${holderFile}" ]; then command rm -f "${lockPath}"; command ln -s "${ownerToken}" "${lockPath}"; fi; command rm "$@"; }\n`); + const swap = bashEnv(dir, `rm() { if [ "$*" = "-f ${holderFile}" ]; then command rm -rf "${lockPath}"; command mkdir "${lockPath}"; printf '%s\\n' "${ownerToken}" > "${lockPath}/owner"; fi; command rm "$@"; }\n`); const r = run(['live', '--root', dir, '--exclude-ancestry-of', String(owner)], { BASH_ENV: swap }); expect(r.stderr).toContain(`reclaimed stale lease ${holder}`); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + expect(ownerOf(lockPath)).toBe(ownerToken); }); + test('a lock held by a running gstack-lease excludes a second taker, which names it; both leases land once it is released', async () => { + const { dir, leases } = checkout(); + const [first, second] = [sleeper(), sleeper()]; + // The first touch holds the lock across its prune (find runs under the lock) until the test releases it. + const release = path.join(dir, 'release'); + const slow = bashEnv(dir, `find() { case "$*" in *'+120'*) until [ -e "${release}" ]; do sleep 0.05; done ;; esac; command find "$@"; }\n`); + const holder = spawn('bash', [LEASE, 'touch', '--root', dir, '--pid', String(first)], { env: { ...baseEnv, BASH_ENV: slow }, stdio: 'ignore' }); + children.push(holder); + const exited = new Promise((resolve) => holder.on('exit', resolve)); + const owner = path.join(leases, '.lock', 'owner'); + for (let i = 0; i < 500 && !fs.existsSync(owner); i++) await Bun.sleep(20); + expect(fs.readFileSync(owner, 'utf8')).toMatch(new RegExp(`^${holder.pid} `)); + const blocked = run(['touch', '--root', dir, '--pid', String(second)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + fs.writeFileSync(release, ''); + expect(blocked.status, blocked.stdout + blocked.stderr).toBe(3); + expect(blocked.stderr).toContain(`locked by live process ${holder.pid};`); + expect(await exited).toBe(0); + expect(run(['touch', '--root', dir, '--pid', String(second)]).status).toBe(0); + expect(fs.readdirSync(leases).sort()).toEqual([String(first), String(second)].sort()); + }, 30_000); + + test('parallel touches all take the lock in turn and leave no lock, claim or temp file behind', async () => { + const { dir, leases } = checkout(); + const pids = Array.from({ length: 16 }, (_, i) => 4300 + i); + // The prune runs under the lock: a second process inside it at the same time logs an overlap. + const inside = path.join(dir, 'inside'); + const overlaps = path.join(dir, 'overlaps.log'); + const probe = bashEnv(dir, `find() { case "$*" in *'+120'*) command mkdir "${inside}" 2>/dev/null || echo overlap >> "${overlaps}"; sleep 0.05; command rmdir "${inside}" 2>/dev/null ;; esac; command find "$@"; }\n`); + const codes = await Promise.all(pids.map((p) => new Promise((resolve) => { + const c = spawn('bash', [LEASE, 'touch', '--root', dir, '--pid', String(p)], { env: { ...baseEnv, BASH_ENV: probe }, stdio: 'ignore' }); + children.push(c); + c.on('exit', resolve); + }))); + expect(codes).toEqual(pids.map(() => 0)); + expect(fs.existsSync(overlaps), 'two touches were inside the lock at once').toBe(false); + expect(fs.readdirSync(leases).sort()).toEqual(pids.map(String).sort()); + }, 30_000); + + // #3330: a taker killed between its mkdir and its owner rename leaves an ownerless lock. + test('an ownerless lock is broken once every claimant is gone, and kept while a live claimant may still own it', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const lockPath = path.join(leases, '.lock'); + fs.mkdirSync(lockPath, { recursive: true }); + age(lockPath); + fs.writeFileSync(path.join(leases, `.lock.claim.${holder}`), `${holder} \n`); + const kept = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(kept.status, kept.stdout + kept.stderr).toBe(3); + expect(kept.stderr).toContain('cannot take the lease directory lock'); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + expect(fs.readdirSync(lockPath)).toEqual([]); + + const dead = deadPid(); + fs.renameSync(path.join(leases, `.lock.claim.${holder}`), path.join(leases, `.lock.claim.${dead}`)); + fs.writeFileSync(path.join(leases, `.lock.claim.${dead}`), `${dead} \n`); + fs.writeFileSync(path.join(leases, '.lock.claim.1'), ''); + age(lockPath); + const broken = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(broken.status, broken.stdout + broken.stderr).toBe(0); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); + expect(fs.existsSync(lockPath)).toBe(false); + + fs.rmSync(path.join(leases, `.lock.claim.${dead}`)); + fs.mkdirSync(lockPath); + age(lockPath); + const unclaimed = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(unclaimed.status, unclaimed.stdout + unclaimed.stderr).toBe(0); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + // Value: protects=an ownerless lock younger than 2 s is never broken, so a live taker whose claim landed after the breaker read the claims keeps + // the lock it is about to publish; a malformed lock (empty owner, pid 0, or a stray file) is broken once old instead of wedging every touch; + // fails_when=orphaned() drops its age check, or treats only a missing owner file (not an empty or invalid one) as ownerless; + // why_new=the ownerless cases above all backdate the lock; nothing pins the young-lock guard or the malformed shapes; + // seam=none; no_claim=the claim-read race itself (a young unclaimed lock stands in for a taker whose claim the breaker missed) + test('an ownerless lock is broken only once it is 2 s old, and a malformed lock is treated as ownerless', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const lockPath = path.join(leases, '.lock'); + fs.mkdirSync(lockPath, { recursive: true }); + // The clock reads the lock's own mtime, so the lock stays 0 s old however slow the runner is. + const frozen = bashEnv(dir, `date() { case "$1" in +%s) command stat -c %Y "${lockPath}" 2>/dev/null || command stat -f %m "${lockPath}" ;; *) command date "$@" ;; esac; }\n`); + const young = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3', BASH_ENV: frozen }); + expect(young.status, young.stdout + young.stderr).toBe(3); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + 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, ''); }], + ]; + for (const [shape, plant] of shapes) { + fs.rmSync(lockPath, { recursive: true, force: true }); + fs.rmSync(path.join(leases, String(holder)), { force: true }); + plant(); + age(lockPath); + if (shape.includes('clock skew')) { + const hourAhead = new Date(Date.now() + 3_600_000); + fs.utimesSync(lockPath, hourAhead, hourAhead); + } + const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(r.status, `${shape}: ${r.stdout}${r.stderr}`).toBe(0); + expect(fs.readdirSync(leases), shape).toEqual([String(holder)]); + } + }); + + // 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('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'); + fs.mkdirSync(lockPath, { recursive: true }); + age(lockPath); + const claim = path.join(leases, `.lock.claim.${reused}`); + fs.writeFileSync(claim, `${reused} \n`); + const fiveMinutesAgo = new Date(Date.now() - 300_000); + fs.utimesSync(claim, fiveMinutesAgo, fiveMinutesAgo); + 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 + // GSTACK_LEASE_LOCK_TRIES falls back to the default instead of looping forever, and a visibly held lock is never mkdir'd; + // fails_when=lock() loses its SECONDS deadline or its tries validation, or take() stops checking the path first; + // why_new=every live-holder case lowers the tries; none runs the default budget, a bad value or counts mkdir calls; + // seam=none; no_claim=real elapsed time (a sleep shim advances SECONDS 10 s per wait) + test('a live holder bounds touch by a 30 s deadline, and a non-numeric tries value cannot loop forever', () => { + const { dir, leases } = checkout(); + const [holder, owner] = [sleeper(), sleeper()]; + plantLock(path.join(leases, '.lock'), `${owner} `); + const waits = path.join(dir, 'waits.log'); + const mkdirs = path.join(dir, 'mkdirs.log'); + const shim = bashEnv(dir, `sleep() { echo "$*" >> "${waits}"; SECONDS=$((SECONDS + 10)); }\nmkdir() { case "$*" in *"/.lock") echo "$*" >> "${mkdirs}" ;; esac; command mkdir "$@"; }\n`); + for (const tries of [undefined, 'abc']) { + fs.rmSync(waits, { force: true }); + const env: Record = { BASH_ENV: shim }; + if (tries) env.GSTACK_LEASE_LOCK_TRIES = tries; + const r = spawnSync('bash', [LEASE, 'touch', '--root', dir, '--pid', String(holder)], { encoding: 'utf8', env: { ...baseEnv, ...env }, timeout: 20_000 }); + expect(r.error?.message ?? '', `tries=${tries}: touch did not end`).not.toContain('ETIMEDOUT'); + expect(r.status, `tries=${tries}: ${r.stdout}${r.stderr}`).toBe(3); + expect(r.stderr).toContain(`locked by live process ${owner};`); + expect(r.stderr).not.toContain('integer expression'); + expect(fs.readFileSync(waits, 'utf8').trim().split('\n').length, `tries=${tries}`).toBeLessThanOrEqual(3); + } + expect(fs.existsSync(mkdirs), 'take() ran mkdir on a visibly held lock').toBe(false); + expect(ownerOf(path.join(leases, '.lock'))).toBe(String(owner)); + }, 30_000); + + // Value: protects=a holder that resumes past the 120-minute prune window keeps its own lock metadata, so unlock still recognizes and removes it; + // fails_when=touch's prune recurses into .lock (deleting owner) or matches lock files at the top level; + // why_new=the prune test plants only lease files; none ages the lock's owner file before the prune runs; + // seam=none; no_claim=a real suspension (a BASH_ENV find shim backdates the owner file just before the prune) + test('the 120-minute prune never deletes the lock it runs under', () => { + 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)]); + }); + + // Value: protects=once a lock is renamed aside it is released: a failed delete of the renamed copy never removes a lock a new taker made at .lock; + // fails_when=drop() reports failure after a successful rename, so unlock's in-place rm -rf fallback runs on the new taker's lock; + // why_new=the rename-aside tests fail the rename itself; none lets the rename succeed and the delete fail; + // seam=none; no_claim=a real new taker in that window (the rm shim plants one, then fails like a Windows file in use) + test('a renamed-aside lock that cannot be deleted never takes a new taker\'s lock with it', () => { + const { dir, leases } = checkout(); + const [holder, taker] = [sleeper(), sleeper()]; + const lockPath = path.join(leases, '.lock'); + const shim = bashEnv(dir, `rm() { case "$*" in *".lock.gone."*) command mkdir "${lockPath}" && printf '%s \\n' "${taker}" > "${lockPath}/owner"; return 1 ;; esac; command rm "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(ownerOf(lockPath)).toBe(String(taker)); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); + }); + + test('a lock that cannot be taken while nobody holds it fails at once instead of waiting out the tries', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const waits = path.join(dir, 'waits.log'); + const broken = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") return 1 ;; esac; command mkdir "$@"; }\nsleep() { echo "$*" >> "${waits}"; command sleep "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: broken }); + expect(r.status, r.stdout + r.stderr).toBe(3); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(r.stderr).toContain('cannot take the lease directory lock'); + expect(r.stderr).not.toContain('locked by live process'); + expect(fs.readdirSync(leases)).toEqual([]); + }); + + // Value: protects=a .lock.break held by a live breaker (owner, or claim on an ownerless dir) is never cleared; an ownerless one with no claimant is; + // fails_when=break_lock's liveness check on .lock.break is dropped, or orphaned() ignores a live claim; + // why_new=the existing .lock.break test plants only a dead breaker's owner, never a live breaker or an ownerless one; + // seam=none; no_claim=two live breakers racing for real (sleeping children stand in as the live holders) + test('a .lock.break whose breaker lives (owner or claim) is never cleared; an ownerless one with no live claimant is', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const breaker = sleeper(); + const lockPath = path.join(leases, '.lock'); + const breakPath = path.join(leases, '.lock.break'); + plantLock(lockPath, `${deadPid()} `); + plantLock(breakPath, `${breaker} `); + const owned = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(owned.status, owned.stdout + owned.stderr).toBe(3); + expect(ownerOf(breakPath)).toBe(String(breaker)); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + fs.rmSync(breakPath, { recursive: true }); + fs.mkdirSync(breakPath); + age(breakPath); + fs.writeFileSync(path.join(leases, `.lock.break.claim.${breaker}`), `${breaker} \n`); + const claimed = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(claimed.status, claimed.stdout + claimed.stderr).toBe(3); + expect(fs.readdirSync(breakPath)).toEqual([]); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + fs.rmSync(path.join(leases, `.lock.break.claim.${breaker}`)); + age(breakPath); + const orphaned = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '5' }); + expect(orphaned.status, orphaned.stdout + orphaned.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=a lock dir whose owner rename failed is removed, never left ownerless; touch exits 3 saying it cannot take the lock; + // fails_when=take() stops removing the lock dir after a failed owner rename; + // why_new=the ownerless-lock test plants the dir by hand; none fails take() between its mkdir and its owner rename; + // seam=none; no_claim=real mv failure modes (a BASH_ENV shim fails the claim-to-owner rename) + test('a lock whose owner cannot be published is removed, not left ownerless, and touch fails at once', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const shim = bashEnv(dir, 'mv() { case "$*" in *".lock.claim."*) return 1 ;; esac; command mv "$@"; }\n'); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(3); + expect(r.stderr).toContain('cannot take the lease directory lock'); + expect(r.stderr).not.toContain('locked by live process'); + expect(fs.readdirSync(leases)).toEqual([]); + }); + + // Value: protects=a lock that vanishes between a failed take and its check is retaken at once: touch writes its lease, no sleep, no lock left; + // fails_when=the vanished-lock retry limit drops to 1, so one release race exits 3; + // why_new=the fails-at-once test pins only the upper bound (every take fails); no test fails a single take; + // seam=none; no_claim=a real release race (a BASH_ENV shim fails the first mkdir of the lock) + test('a lock released between a failed take and its check is retaken without waiting', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const waits = path.join(dir, 'waits.log'); + const once = path.join(dir, 'failed-once'); + const shim = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") if [ ! -e "${once}" ]; then : > "${once}"; return 1; fi ;; esac; command mkdir "$@"; }\nsleep() { echo "$*" >> "${waits}"; command sleep "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.existsSync(once), 'the shim never failed a take').toBe(true); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=unlock removes the caller's own lock even when renaming it aside fails: touch exits 0, lease written, no .lock left behind; + // fails_when=the rm -rf fallback after drop in unlock() is removed; + // why_new=no test fails the rename-aside step of drop(), so unlock's fallback never runs; + // seam=none; no_claim=real mv failure modes such as a Windows file in use (a BASH_ENV shim fails the .lock.gone rename) + test('unlock still removes its own lock when renaming it aside fails', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const shim = bashEnv(dir, 'mv() { case "$*" in *".lock.gone."*) return 1 ;; esac; command mv "$@"; }\n'); + 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)]); + }); + + // Value: protects=a dead holder's lock that cannot be renamed aside is removed in place, and one that cannot be removed at all ends touch + // with exit 3 within the tries limit instead of spinning, without calling the dead holder live; + // fails_when=break_lock reports success after a failed drop and lock() loops without sleeping or counting tries; + // why_new=the dead-holder tests always let the rename-aside succeed; the tries limit is only reached through the live-holder path; + // seam=none; no_claim=real mv/rm failure modes such as a Windows file in use (a BASH_ENV shim fails the .lock.gone rename and the rm) + test('a dead holder\'s lock that cannot be renamed aside ends touch with a verdict instead of spinning', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const dead = deadPid(); + const touch = (shim: string) => spawnSync('bash', [LEASE, 'touch', '--root', dir, '--pid', String(holder)], { + encoding: 'utf8', env: { ...baseEnv, BASH_ENV: shim, GSTACK_LEASE_LOCK_TRIES: '3' }, timeout: 8_000, + }); + const noRename = 'mv() { case "$*" in *".lock.gone."*) return 1 ;; esac; command mv "$@"; }\n'; + plantLock(path.join(leases, '.lock'), `${dead} `); + const removed = touch(bashEnv(dir, noRename)); + expect(removed.error?.message ?? '', 'touch did not end within 8 s').not.toContain('ETIMEDOUT'); + expect(removed.status, removed.stdout + removed.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + + fs.rmSync(path.join(leases, String(holder))); + plantLock(path.join(leases, '.lock'), `${dead} `); + const stuck = touch(bashEnv(dir, noRename + 'rm() { case "$*" in *"/.lock") return 1 ;; esac; command rm "$@"; }\n')); + expect(stuck.error?.message ?? '', 'touch did not end within 8 s').not.toContain('ETIMEDOUT'); + expect(stuck.status, stuck.stdout + stuck.stderr).toBe(3); + expect(stuck.stderr).toContain('cannot take the lease directory lock'); + expect(stuck.stderr).not.toContain('locked by live process'); + expect(ownerOf(path.join(leases, '.lock'))).toBe(String(dead)); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + }, 20_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(); @@ -333,7 +698,7 @@ describe('gstack-lease', () => { fs.writeFileSync(path.join(state, 'config.yaml'), 'update_check: false\n'); const leaseDir = path.join(state, 'leases'); fs.mkdirSync(leaseDir); - fs.symlinkSync(`${sleeper()} `, path.join(leaseDir, '.lock')); + plantLock(path.join(leaseDir, '.lock'), `${sleeper()} `); const r = spawnSync(START, ['--skill', 'testskill', '--parent-pid', '424244'], { encoding: 'utf8', cwd: home, env: { PATH: process.env.PATH!, HOME: home, GSTACK_HOME: state, GSTACK_LEASE_DIR: leaseDir, GSTACK_LEASE_LOCK_TRIES: '2' }, timeout: 30_000, });