From c23d7d48fe273e54776192f23c4f272ce6ab4d2d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 25 Oct 2026 15:45:05 -0700 Subject: [PATCH] fix(session-update): the starting session's own lease never skips its update; autostash fixtures own their lease dir and state root (GSTA-91) (#3412) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 14 ++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-session-update | 9 ++- package.json | 2 +- test/session-update-autostash.test.ts | 100 ++++++++++++++++++++++---- test/session-update-stages.test.ts | 15 ++++ 7 files changed, 126 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58db930b9..ba97bc0e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## [1.91.221.0] - 2026-10-25 + +**The session-start updater no longer skips because of its own session's workflow lease, and the autostash tests stop failing their first pass in Free Tests.** Since #3385, `gstack-session-update` skips the fast-forward while another session holds a workflow lease, and it means to ignore the session that started it (`--exclude-ancestry-of`). The check runs in the forked background updater, though, and passed the hook's own `$$`, which has exited by then, so the ancestry walk excluded nothing. A Claude Code process that had already run a skill (and so held a lease) and then started a session again in the same process (`/clear`, resume or compaction) logged `SKIP leased pid=` and never updated. `test/session-update-autostash.test.ts` hit the same gap plus a fixture leak: its first two fixtures spawned the updater with the whole parent environment, so it read the test preload's `GSTACK_LEASE_DIR`, which every test file in the same Bun process shares. A lease another file left there gave "Received: SKIP leased pid=3196/1m" where UPDATING or PULL_FAILED was expected, in 13 Free Tests runs since #3385 merged (each passed on the serial retry). + +### Fixed +- **`bin/gstack-session-update`**: before it forks, the hook resolves its caller's harness pid with `gstack_harness_pid "$PPID"` (`bin/gstack-harness-pid.sh`), the same derivation `bin/gstack-skill-start` uses for the pid it leases, and the background lease check excludes that pid's ancestry. Leases held by other live sessions still skip the update. `bin/gstack-upgrade` runs its check in the foreground, so its `$$` is alive and it is unchanged. + +### Tests +- `test/session-update-autostash.test.ts`: every fixture now runs the updater with its own `GSTACK_STATE_ROOT` (the top of the state-root chain, instead of `GSTACK_STATE_DIR`, which an inherited `GSTACK_HOME` outranks) and its own `GSTACK_LEASE_DIR`. A negative control holds a live lease in the shared preload lease directory, confirms the ambient environment sees it, and shows the fixture still updates; it fails with "SKIP leased" without the fixture change. A positive case shows the fixture's own live lease still skips the update. +- `test/session-update-stages.test.ts`: the starting session's own lease (the test process, behind a `/bin/sh -c` wrapper as a hook runner would use) no longer blocks the update. It fails on main with `SKIP leased`. The other session-update test files already pass an explicit environment, so their leases resolve to the fixture install's own git dir. + +### Gate integrity +- No threshold, budget, timeout, trial count, case kind or assertion changed. The lease rule is narrower in one respect only: the starting session's own harness and its ancestors no longer count as holders, which is what `--exclude-ancestry-of` promised. + ## [1.91.220.0] - 2026-10-25 **A design doc edited in host plan mode no longer goes stale for the plan reviews (#542).** When /office-hours ran inside Claude Code's plan mode, the design doc lived twice: gstack's copy under `~/.gstack/projects//` and the host's plan file under `.claude/plans/`. Later edits went only to the plan file, so /plan-ceo-review and /plan-eng-review read the older gstack copy and called it the source of truth. The gstack design doc stays the single source (Garry, Oct 17: "designdoc"). Edits made in the plan copy are now written through to it, and every plan review is warned when the copy is newer. diff --git a/VERSION b/VERSION index 01a0bafe4..13b9186ef 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.220.0 +1.91.221.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 7aacd0da4..a582bf0f8 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.220.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.221.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-session-update b/bin/gstack-session-update index 2ae01297f..a3fa3631c 100755 --- a/bin/gstack-session-update +++ b/bin/gstack-session-update @@ -252,6 +252,13 @@ if [ -f "$THROTTLE_FILE" ]; then fi fi +# The caller's own session never counts as a lease holder (#3363). Resolve its +# harness pid now, the same way bin/gstack-skill-start names the pid it leases: +# once this hook exits, the background updater's $$ names a dead process and +# an ancestry walk from it excludes nothing. +. "$(cd "$(dirname "$0")" && pwd)/gstack-harness-pid.sh" 2>/dev/null || gstack_harness_pid() { printf '%s\n' "$1"; } +CALLER_PID=$(gstack_harness_pid "$PPID") + # ── Fork to background: zero latency on session start ── ( # Prevent git from prompting for credentials (would hang the background process) @@ -368,7 +375,7 @@ fi # retry runs under it, the rule bin/gstack-upgrade enforces as UPGRADE_BUSY. # The throttle file is not written, so the next session start tries again. if [ "${GSTACK_UPGRADE_FORCE:-}" != 1 ] && [ -x "$LEASE" ] \ - && BUSY=$("$LEASE" live --root "$GSTACK_DIR" --exclude-ancestry-of $$ 2>/dev/null); then + && BUSY=$("$LEASE" live --root "$GSTACK_DIR" --exclude-ancestry-of "$CALLER_PID" 2>/dev/null); then log_entry "SKIP leased $(printf '%s\n' "$BUSY" | awk 'NF { printf "%spid=%s/%s", n++ ? " " : "", $1, $2 }')" exit 0 fi diff --git a/package.json b/package.json index 2d089b97d..0b396bbf1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.220", + "version": "1.91.221", "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/session-update-autostash.test.ts b/test/session-update-autostash.test.ts index 739cb600b..f2ab223d9 100644 --- a/test/session-update-autostash.test.ts +++ b/test/session-update-autostash.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, test, expect } from 'bun:test'; -import { execFileSync, spawnSync } from 'child_process'; +import { execFileSync, spawn, spawnSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -25,7 +25,7 @@ function makeFixture() { const install = path.join(base, 'install'); const state = path.join(base, 'state'); fs.mkdirSync(state, { recursive: true }); - execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin]); + execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 30_000 }); fs.mkdirSync(path.join(seed, 'bin'), { recursive: true }); fs.writeFileSync(path.join(seed, 'VERSION'), '1.0.0\n'); @@ -45,14 +45,22 @@ function makeFixture() { git(seed, 'branch', '-M', 'main'); git(seed, 'remote', 'add', 'origin', origin); git(seed, 'push', '-q', 'origin', 'main'); - execFileSync('git', ['clone', '-q', origin, install]); + execFileSync('git', ['clone', '-q', origin, install], { timeout: 30_000 }); return { base, origin, seed, install, state }; } -function runScript(install: string, state: string) { +// Each fixture owns its state root (GSTACK_STATE_ROOT tops the chain in +// bin/gstack-state-root.sh) and its workflow-lease directory. The preload's +// GSTACK_LEASE_DIR is shared by every test file in this Bun process, so a lease +// another file leaves there made the updater log SKIP leased (#3385). +function fixtureEnv(base: string, install: string, state: string) { + return { ...process.env, GSTACK_DIR: install, GSTACK_STATE_ROOT: state, GSTACK_LEASE_DIR: path.join(base, 'leases') }; +} + +function runScript(base: string, install: string, state: string) { return spawnSync('bash', [SCRIPT], { encoding: 'utf8', - env: { ...process.env, GSTACK_DIR: install, GSTACK_STATE_DIR: state }, + env: fixtureEnv(base, install, state), timeout: 20000, }); } @@ -85,7 +93,7 @@ describe('gstack-session-update pull wedge (#2566)', () => { // prefix-rename / gbrain-block shape: tracked file, modified). fs.appendFileSync(path.join(install, 'SKILL.md'), 'locally patched line\n'); - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /UPDATING|UP_TO_DATE|PULL_FAILED/); expect(log).not.toContain('PULL_FAILED'); @@ -110,7 +118,7 @@ describe('gstack-session-update pull wedge (#2566)', () => { git(seed, 'commit', '-aqm', 'remote divergence'); git(seed, 'push', '-q', 'origin', 'main'); - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /PULL_FAILED/); expect(log).toContain('PULL_FAILED'); @@ -123,6 +131,70 @@ describe('gstack-session-update pull wedge (#2566)', () => { }, 30000); }); +describe.skipIf(process.platform === 'win32')('gstack-session-update fixtures ignore leases outside their own (#3385)', () => { + const LEASE = path.join(ROOT, 'bin', 'gstack-lease'); + const sleepers: ReturnType[] = []; + afterEach(() => { for (const s of sleepers.splice(0)) s.kill('SIGKILL'); }); + + function holdLease(install: string, leaseDir: string): number { + const sleeper = spawn('sleep', ['120'], { stdio: 'ignore' }); + sleepers.push(sleeper); + const touch = spawnSync('bash', [LEASE, 'touch', '--root', install, '--pid', String(sleeper.pid)], { + encoding: 'utf8', env: { ...process.env, GSTACK_LEASE_DIR: leaseDir }, timeout: 10_000, + }); + expect(touch.status, touch.stderr).toBe(0); + return sleeper.pid!; + } + + function advanceUpstream(seed: string): string { + fs.writeFileSync(path.join(seed, 'VERSION'), '1.1.0\n'); + git(seed, 'commit', '-aqm', 'upstream release'); + git(seed, 'push', '-q', 'origin', 'main'); + return git(seed, 'rev-parse', 'HEAD'); + } + + test('a live lease in the shared preload lease directory does not stop the fixture update (negative control)', async () => { + const shared = process.env.GSTACK_LEASE_DIR!; + expect(shared).toBeTruthy(); + const { base, seed, install, state } = makeFixture(); + let pid = 0; + try { + const upstreamHead = advanceUpstream(seed); + pid = holdLease(install, shared); + const ambient = spawnSync('bash', [LEASE, 'live', '--root', install], { encoding: 'utf8', env: process.env, timeout: 10_000 }); + expect(ambient.status).toBe(0); + expect(ambient.stdout).toContain(`${pid} `); + + expect(runScript(base, install, state).status).toBe(0); + const log = await waitForLog(state, /UPDATING|PULL_FAILED|SKIP /); + expect(log).not.toContain('SKIP leased'); + expect(log).toContain('UPDATING'); + expect(git(install, 'rev-parse', 'HEAD')).toBe(upstreamHead); + } finally { + if (pid) fs.rmSync(path.join(shared, String(pid)), { force: true }); + fs.rmSync(base, { recursive: true, force: true }); + } + }, 30000); + + test("the fixture's own live lease still skips the update", async () => { + const { base, seed, install, state } = makeFixture(); + try { + const before = git(install, 'rev-parse', 'HEAD'); + advanceUpstream(seed); + const pid = holdLease(install, path.join(base, 'leases')); + + expect(runScript(base, install, state).status).toBe(0); + const log = await waitForLog(state, /SKIP leased /); + expect(log).toMatch(new RegExp(`SKIP leased pid=${pid}/\\d+m`)); + expect(log).not.toContain('UPDATING'); + expect(git(install, 'rev-parse', 'HEAD')).toBe(before); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }, 30000); +}); + + // ── #2613: the lock pidfile must record the LIVE holder, not the exited parent ── // // `echo $$` inside the backgrounded subshell recorded the parent hook's PID. @@ -144,10 +216,10 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { return shimDir; } - function runScriptWithPath(install: string, state: string, shimDir: string) { + function runScriptWithPath(base: string, install: string, state: string, shimDir: string) { return spawnSync('bash', [SCRIPT], { encoding: 'utf8', - env: { ...process.env, GSTACK_DIR: install, GSTACK_STATE_DIR: state, PATH: `${shimDir}:${process.env.PATH}` }, + env: { ...fixtureEnv(base, install, state), PATH: `${shimDir}:${process.env.PATH}` }, timeout: 20000, }); } @@ -160,7 +232,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const { base, install, state } = makeFixture(); const shimDir = makeSlowGitShim(base, 3); try { - const r = runScriptWithPath(install, state, shimDir); + const r = runScriptWithPath(base, install, state, shimDir); expect(r.status).toBe(0); // parent hook has EXITED by now (spawnSync waited) // Poll for the pidfile the detached subshell writes. const pidPath = path.join(state, '.setup-lock', 'pid'); @@ -191,7 +263,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const lockDir = path.join(state, '.setup-lock'); fs.mkdirSync(lockDir, { recursive: true }); fs.writeFileSync(path.join(lockDir, 'pid'), String(holder.pid)); - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /SKIP locked_by=/); expect(log).toContain(`SKIP locked_by=${holder.pid}`); @@ -209,7 +281,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const lockDir = path.join(state, '.setup-lock'); fs.mkdirSync(lockDir, { recursive: true }); fs.writeFileSync(path.join(lockDir, 'pid'), String(dead.pid)); - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /UP_TO_DATE|UPDATING/); expect(log).toMatch(/UP_TO_DATE|UPDATING/); @@ -224,7 +296,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const lockDir = path.join(state, '.setup-lock'); fs.mkdirSync(lockDir, { recursive: true }); fs.writeFileSync(path.join(lockDir, 'pid'), ''); // mkdir→echo race window - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /SKIP locked_by=/); expect(log).toContain('SKIP locked_by='); @@ -290,7 +362,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { // forever, so liveness alone can never clear this wedge. const past = new Date(Date.now() - 40 * 60 * 1000); fs.utimesSync(pidPath, past, past); - const r = runScript(install, state); + const r = runScript(base, install, state); expect(r.status).toBe(0); const log = await waitForLog(state, /RECLAIMED lock_ttl_expired/); expect(log).toContain('RECLAIMED lock_ttl_expired'); diff --git a/test/session-update-stages.test.ts b/test/session-update-stages.test.ts index 7bace11a9..a5e5f0f86 100644 --- a/test/session-update-stages.test.ts +++ b/test/session-update-stages.test.ts @@ -364,5 +364,20 @@ describe.skipIf(process.platform === 'win32')('gstack-session-update honors work const log = await waitFor(dead, /UPDATED from=1\.0\.0 to=1\.1\.0/); expect(log).not.toContain('SKIP leased'); }, 30_000); + + test("the starting session's own lease never blocks its update, behind a shell wrapper too", async () => { + const fx = makeFixture(); + upstreamRelease(fx, '1.1.0'); + touchLease(fx, process.pid); + const r = spawnSync('/bin/sh', ['-c', 'bash "$0"; exit $?', SCRIPT], { + encoding: 'utf8', + env: { PATH: `${path.dirname(process.execPath)}:${process.env.PATH}`, HOME: fx.home, GSTACK_DIR: fx.install, GSTACK_STATE_ROOT: fx.state, SETUP_CONTROL: fx.control, SETUP_CALLS: fx.calls, TMPDIR: fx.base, ...leaseEnv(fx) }, + timeout: 20_000, + }); + expect(r.status, r.stderr).toBe(0); + const log = await waitFor(fx, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(log).not.toContain('SKIP leased'); + expect(setupRuns(fx)).toBe(1); + }, 30_000); });