From 76f67dbcdbc7d23c51b12cd69f396040b6fe2226 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:58:30 +0000 Subject: [PATCH] fix: a hold without --run survives a sibling's release, and an orphaned run ages out on its own (GST-1205 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-lease | 38 +++++++++++++++++++-------- changelog.d/gst-1205-upgrade-lease.md | 2 +- test/gstack-lease.test.ts | 28 ++++++++++++++++++++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/bin/gstack-lease b/bin/gstack-lease index 46529c26f..f4007f85a 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -10,8 +10,8 @@ # Usage: # gstack-lease touch [--root ] [--pid

| --from ] [--run ] # write /gstack-leases/ holding the pid's start identity -# and the open run (gstack-skill-start's SESSION_ID); a lease with -# no open run (written without --run, or by an older gstack) never idles; +# and the open run (gstack-skill-start's SESSION_ID); without --run +# it records a hold no release closes; a run or hold counts 120 min; # pid defaults to the harness pid behind this call # (bin/gstack-harness-pid.sh, the same derivation the session marker # uses), or behind --from (a caller whose own parent is short-lived @@ -254,6 +254,15 @@ reclaim() { unlock return 1 } +# Run entries (GST-1205), lines after the identity: `run ` per +# open run, `hold ` for a holder that opened none, `idle` once every +# run closed. An entry older than 120 minutes no longer counts. +# fresh FILE [DROP] — the entries under 120 minutes old, minus the line whose +# first two fields are DROP ("run " or "hold"). +fresh() { sed -n '2,$p' "$1" 2>/dev/null | awk -v now="$(date +%s)" -v drop="${2:-}" '($1 == "run" && NF == 3 && now - $3 < 7200 && $1 " " $2 != drop) || ($1 == "hold" && NF == 2 && now - $2 < 7200 && $1 != drop)'; } +# idle FILE — the lease records entries and none of them still counts. A lease +# with no entries (written without --run, or by an older gstack) is never idle. +idle() { [ -n "$(sed -n '2,$p' "$1" 2>/dev/null | grep -v '^$')" ] && [ -z "$(fresh "$1")" ]; } age_min() { # FILE — minutes since its mtime _m=$(stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null || echo 0) echo $(( ( $(date +%s) - _m ) / 60 )) @@ -270,12 +279,19 @@ case "$VERB" in exit 3 } _tmp="$DIR/.$PID.$$" - _id=$(identity "$PID") _runs="" - [ "$(head -1 "$DIR/$PID" 2>/dev/null)" = "$_id" ] && _runs=$(sed -n '2,$p' "$DIR/$PID" 2>/dev/null | grep '^run ' | grep -vxF "run ${RUN:--}") + _id=$(identity "$PID") _keep="" _held=0 _now=$(date +%s) + if [ -f "$DIR/$PID" ] && [ "$(head -1 "$DIR/$PID" 2>/dev/null)" = "$_id" ]; then + if [ -n "$RUN" ]; then _keep=$(fresh "$DIR/$PID" "run $RUN"); else _keep=$(fresh "$DIR/$PID" hold); fi + [ -z "$(sed -n '2,$p' "$DIR/$PID" 2>/dev/null | grep -v '^$')" ] && _held=1 + fi { printf '%s\n' "$_id" - [ -n "$_runs" ] && printf '%s\n' "$_runs" - [ -z "$RUN" ] || printf 'run %s\n' "$RUN" + [ -z "$_keep" ] || printf '%s\n' "$_keep" + if [ -n "$RUN" ]; then + [ "$_held" -eq 1 ] && echo "hold $_now" + echo "run $RUN $_now" + elif [ -n "$_keep" ]; then echo "hold $_now" + fi } > "$_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 for _c in "$DIR"/.lock*.claim.*; do @@ -292,11 +308,11 @@ case "$VERB" in [ -f "$DIR/$PID" ] || exit 0 lock || exit 3 f="$DIR/$PID" _tmp="$DIR/.$PID.$$" - if grep -qxF "run $RUN" "$f" 2>/dev/null; then + if sed -n '2,$p' "$f" | awk -v id="$RUN" '$1 == "run" && $2 == id { found = 1 } END { exit !found }'; then + _keep=$(fresh "$f" "run $RUN") { head -1 "$f" - sed -n '2,$p' "$f" | grep -vxF "run $RUN" - sed -n '2,$p' "$f" | grep -vxF "run $RUN" | grep -q '^run ' || echo idle + if [ -n "$_keep" ]; then printf '%s\n' "$_keep"; else echo idle; fi } > "$_tmp" && touch -r "$f" "$_tmp" && mv -f "$_tmp" "$f" || { rm -f "$_tmp"; unlock; exit 2; } fi unlock @@ -307,7 +323,7 @@ case "$VERB" in p=${f##*/} _age=$(age_min "$f") _st=$(state "$p" "$(head -1 "$f" 2>/dev/null)" "$_age") - [ "$_st" = alive ] && grep -qx idle "$f" 2>/dev/null && _st=idle + [ "$_st" = alive ] && idle "$f" && _st=idle echo "$p ${_age}m $_st" done exit 0 ;; @@ -325,7 +341,7 @@ done FOUND=1 for f in "$DIR"/*; do [ -f "$f" ] || continue - grep -qx idle "$f" 2>/dev/null && continue + idle "$f" && continue p=${f##*/} _age=$(age_min "$f") case "$ANCESTRY" in *" $p "*) [ "$SHARED" -eq 1 ] && [ "$_age" -lt 120 ] && gstack_shared_harness "$p" || continue ;; esac diff --git a/changelog.d/gst-1205-upgrade-lease.md b/changelog.d/gst-1205-upgrade-lease.md index 5cb734d38..22ec018c0 100644 --- a/changelog.d/gst-1205-upgrade-lease.md +++ b/changelog.d/gst-1205-upgrade-lease.md @@ -1,3 +1,3 @@ ### Fixed -- **A finished /ship no longer blocks `gstack-upgrade` for two hours (GST-1205).** Each `gstack-skill-start` writes a workflow lease that `gstack-upgrade` checks before moving a shared checkout. Nothing released it when the workflow ended, and on Capy every session's lease names the machine's one `capy-server`. So after a /ship finished, its own lease kept refusing the upgrade with UPGRADE_BUSY until it was 120 minutes old, unless you set `GSTACK_UPGRADE_FORCE=1`. Now `gstack-skill-start` records its `SESSION_ID` as an open run on the lease, and `gstack-skill-end` closes it with the new `gstack-lease release`. Once every run on a lease is closed, the lease is idle and blocks nothing (`gstack-lease list` shows `idle`). A run another session still has open keeps blocking, and so does a lease with no recorded run (an older gstack, or a touch without `--run`). A run that never reaches `gstack-skill-end` still ages out after 120 minutes. +- **A finished /ship no longer blocks `gstack-upgrade` for two hours (GST-1205).** Each `gstack-skill-start` writes a workflow lease that `gstack-upgrade` checks before moving a shared checkout. Nothing released it when the workflow ended, and on Capy every session's lease names the machine's one `capy-server`. So after a /ship finished, its own lease kept refusing the upgrade with UPGRADE_BUSY until it was 120 minutes old, unless you set `GSTACK_UPGRADE_FORCE=1`. Now `gstack-skill-start` records its `SESSION_ID` as an open run on the lease, and `gstack-skill-end` closes it with the new `gstack-lease release`. Once every run on a lease is closed, the lease is idle and blocks nothing (`gstack-lease list` shows `idle`). A run another session still has open keeps blocking. So does a holder that records no run (an older gstack, or `gstack-lease touch` without `--run`, which now leaves a `hold` entry that no release removes). A run that never reaches `gstack-skill-end` stops counting 120 minutes after its last renewal, even while other sessions keep renewing the lease. diff --git a/test/gstack-lease.test.ts b/test/gstack-lease.test.ts index 44bf12e51..7850633d2 100644 --- a/test/gstack-lease.test.ts +++ b/test/gstack-lease.test.ts @@ -740,6 +740,34 @@ describe('gstack-lease', () => { expect(run(['live'], env).stdout).toMatch(new RegExp(`^${other} \\d+m sleep$`, 'm')); }); + test('GST-1205 review: a hold without --run survives a sibling run\'s release in either order; an unrecorded release changes nothing; an orphaned run ages out by itself', () => { + const { dir, leases } = checkout(); + const live = (pid: number) => run(['live', '--root', dir]).stdout.includes(`${pid} `); + for (const order of [['hold', 'run'], ['run', 'hold']]) { + const p = sleeper(); + for (const step of order) expect(run(['touch', '--root', dir, '--pid', String(p), ...(step === 'run' ? ['--run', `${p}-1-a`] : [])]).status).toBe(0); + expect(run(['release', '--root', dir, '--pid', String(p), '--run', `${p}-1-a`]).status).toBe(0); + expect(live(p), order.join('>')).toBe(true); + } + const legacy = sleeper(); + expect(run(['touch', '--root', dir, '--pid', String(legacy)]).status).toBe(0); + expect(run(['release', '--root', dir, '--pid', String(legacy), '--run', `${legacy}-9-z`]).status).toBe(0); + expect(live(legacy)).toBe(true); + expect(run(['release', '--root', dir, '--pid', String(legacy)]).status).toBe(2); + expect(run(['release', '--root', dir, '--pid', 'x', '--run', 'y']).status).toBe(2); + const shared = sleeper(); + expect(run(['touch', '--root', dir, '--pid', String(shared), '--run', `${shared}-1-old`]).status).toBe(0); + const file = path.join(leases, String(shared)); + const old = Math.floor(Date.now() / 1000) - 121 * 60; + fs.writeFileSync(file, fs.readFileSync(file, 'utf8').replace(/^run (\S+) \d+$/m, `run $1 ${old}`)); + expect(live(shared)).toBe(false); + expect(run(['touch', '--root', dir, '--pid', String(shared), '--run', `${shared}-2-new`]).status).toBe(0); + expect(live(shared)).toBe(true); + expect(run(['release', '--root', dir, '--pid', String(shared), '--run', `${shared}-2-new`]).status).toBe(0); + expect(live(shared)).toBe(false); + expect(fs.readFileSync(file, 'utf8')).not.toContain('-1-old'); + }); + test('gstack-skill-start reports a lease the held lock kept from being written, and still starts', () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-lease-home-')); const state = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-lease-state-'));