mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
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>
This commit is contained in:
1 parent
956a3f5484
commit
c23d7d48fe
7 files changed
+126
-18
No files matched your search
@@ -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=<its own 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/<slug>/` 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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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<typeof spawn>[] = [];
|
||||
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');
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user