fix(auto-update): hold an update whose hooks do not parse, before the checkout moves

bin/gstack-session-update fast-forwards the live checkout before setup runs,
and Claude Code runs hooks straight from it, so setup's gate cannot protect
an auto-update. After the Bun floor check the updater extracts the incoming
revision with git archive and runs bin/gstack-hook-check on it; a failure
holds the update like bun-too-old, records hook-does-not-parse with file:line
in the update status, and the next session start says the current hooks keep
running. Fixture-remote tests cover a broken shim, a broken imported module
and a parseable control.

Co-authored-by: BenjaminDSmithy <BenjaminDSmithy@users.noreply.github.com>

bin/gstack-session-update

@@ -8,7 +8,12 @@
 # Before the live checkout moves, the updater fetches, reads the incoming
 # release's Bun floor (bin/gstack-bun-version.sh) and checks the `bun` setup
 # will run; below it the update is held with reason bun-too-old and nothing
-# changes (E1). Updaters older than this check cannot protect their first pull.
+# changes (E1). Then it parse-checks every hook the incoming revision registers
+# (bin/gstack-hook-check on a `git archive` of that revision): Claude Code runs
+# hooks straight from this checkout, so an unparseable hook would block tool
+# calls the moment the checkout moved. A failure holds the update with reason
+# hook-does-not-parse and its file:line. Updaters older than a check cannot
+# protect their first pull.
 #
 # Stages (B11): pull, setup and migrations are tracked separately. The
 # "just upgraded" marker is written only when every stage succeeded. A failed
@@ -37,12 +42,28 @@ THROTTLE_FILE="$STATE_DIR/.last-session-update"
 LOCK_DIR="$STATE_DIR/.setup-lock"
 LOG_FILE="$STATE_DIR/analytics/session-update.log"
 PENDING_FILE="$STATE_DIR/session-update-pending"
+HOOK_CHECK="$(cd "$(dirname "$0")" && pwd)/gstack-hook-check"
 THROTTLE_SECONDS=3600  # 1 hour
 
 log_entry() {
   mkdir -p "$(dirname "$LOG_FILE")"
   echo "$(date -u +%Y-%m-%dT%H:%M:%SZ) $1" >> "$LOG_FILE" 2>/dev/null || true  # best-effort: logging never blocks a session
 }
+# hook_incoming_hold <rev> — parse-check the hooks of <rev> (extracted with
+# git archive) before the live checkout moves. Prints the held reason and
+# returns 0 when a hook does not parse; otherwise prints nothing and returns 1
+# (an archive or bun failure fails open, like every other error here).
+hook_incoming_hold() {
+  _hc_dir=$(mktemp -d "${TMPDIR:-/tmp}/gstack-session-hooks-XXXXXX" 2>/dev/null) || return 1
+  _hc_out=""
+  if git -C "$GSTACK_DIR" archive --format=tar "$1" 2>/dev/null | tar -xf - -C "$_hc_dir" 2>/dev/null; then
+    _hc_out=$("$HOOK_CHECK" "$_hc_dir" 2>/dev/null)
+  fi
+  rm -rf "$_hc_dir"
+  _hc_fail=$(printf '%s\n' "$_hc_out" | sed -n 's/^fail [^ ]* //p' | head -1)
+  [ -n "$_hc_fail" ] || return 1
+  printf 'hook-does-not-parse: %s\n' "${_hc_fail#"$_hc_dir"/}" | head -c 300
+}
 pending_get() { sed -n "s/^$1=//p" "$PENDING_FILE" 2>/dev/null | head -1; }
 # pending_save — persist the P_* stage state; nothing pending removes the file.
 pending_save() {
@@ -74,6 +95,8 @@ if [ -f "$PENDING_FILE" ]; then
     case "$_P_PULL" in
       bun-too-old*)
         echo "gstack auto-update: update held ($_P_PULL); nothing was installed or changed. Fix now: bun upgrade && cd $GSTACK_DIR && git pull --ff-only && ./setup" ;;
+      hook-does-not-parse*)
+        echo "gstack auto-update: update held ($_P_PULL); nothing was installed or changed, and your current hooks keep running. This is a gstack bug; report ${_P_PULL#hook-does-not-parse: } at https://github.com/garrytan/gstack/issues. gstack retries at the next check. https://github.com/garrytan/gstack/blob/main/docs/troubleshooting.md#auto-update-hook-does-not-parse" ;;
       *)
         echo "gstack auto-update: git pull did not finish ($_P_PULL); gstack is not updating. Fix now: cd $GSTACK_DIR && git pull --ff-only && ./setup" ;;
     esac
@@ -188,14 +211,17 @@ fi
   # The fast-forward targets the exact revision whose requirement was read.
   # Setup runs `bun` from this PATH, so that is the binary checked; a
   # malformed version passes here because setup only warns on it.
-  INCOMING="" BUN_HELD=""
+  INCOMING="" HELD=""
   if [ "$PULL_EXIT" -eq 0 ]; then
     INCOMING=$(git -C "$GSTACK_DIR" rev-parse --verify '@{upstream}^{commit}' 2>"${PULL_ERR_FILE:-/dev/null}") || PULL_EXIT=1
   fi
   if [ "$PULL_EXIT" -eq 0 ] && [ "$INCOMING" != "$OLD_HEAD" ]; then
-    BUN_HELD=$(gstack_bun_incoming_hold "$GSTACK_DIR" "$INCOMING" 2>/dev/null) || BUN_HELD=""
+    HELD=$(gstack_bun_incoming_hold "$GSTACK_DIR" "$INCOMING" 2>/dev/null) || HELD=""
   fi
-  if [ "$PULL_EXIT" -eq 0 ] && [ -z "$BUN_HELD" ]; then
+  if [ "$PULL_EXIT" -eq 0 ] && [ -z "$HELD" ] && [ "$INCOMING" != "$OLD_HEAD" ]; then
+    HELD=$(hook_incoming_hold "$INCOMING") || HELD=""
+  fi
+  if [ "$PULL_EXIT" -eq 0 ] && [ -z "$HELD" ]; then
     git -C "$GSTACK_DIR" merge --ff-only --autostash -q "$INCOMING" >/dev/null 2>"${PULL_ERR_FILE:-/dev/null}"
     PULL_EXIT=$?
   fi
@@ -212,9 +238,9 @@ fi
   case "$P_FAILURES" in ''|*[!0-9]*) P_FAILURES=0 ;; esac
   case "$P_NEXT" in ''|*[!0-9]*) P_NEXT=0 ;; esac
 
-  if [ -n "$BUN_HELD" ]; then
-    log_entry "HELD $BUN_HELD incoming=$INCOMING"
-    P_PULL="$BUN_HELD"
+  if [ -n "$HELD" ]; then
+    log_entry "HELD $HELD incoming=$INCOMING"
+    P_PULL="$HELD"
     pending_save
     rm -f "$PULL_ERR_FILE" 2>/dev/null
     exit 0

docs/troubleshooting.md

@@ -541,6 +541,19 @@ cd ~/.claude/skills/gstack && ./setup
 
 **Expected result.** Setup finishes with exit 0 and no refusal line.
 
+<a id="auto-update-hook-does-not-parse"></a>
+### `gstack auto-update: update held (hook-does-not-parse: <file>:<line>: <error>); nothing was installed or changed, and your current hooks keep running. ...`
+
+**Meaning.** Team-mode auto-update fetched a release with a hook that does not
+parse. It checked the incoming revision before moving your checkout, so your
+checkout, installed skills and registered hooks stay at the current revision.
+gstack checks again at the next update check and installs the first release
+whose hooks parse. This is a gstack bug: report the printed `<file>:<line>`.
+
+**Fix.** Nothing to do locally. A manual `git pull` followed by `./setup`
+cannot be checked before the pull; setup then refuses the broken hook (see the
+entry above).
+
 <a id="cso-windows-msvc-compile"></a>
 ### `CSO unavailable: its native helper was not built (windows-msvc-compile)`
 

test/session-update-hook-hold.test.ts

@@ -0,0 +1,161 @@
+import { afterEach, describe, expect, test } from 'bun:test';
+import { execFileSync, spawnSync } from 'child_process';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
+// The auto-updater fast-forwards the live checkout before setup runs, and
+// Claude Code runs hooks straight from that checkout, so setup's parse gate
+// alone cannot protect an auto-update. The updater parse-checks every hook of
+// the incoming revision (git archive into a temp dir, bin/gstack-hook-check)
+// before the fast-forward; a failure holds the update like a too-old Bun:
+// the checkout does not move and the installed hooks keep running.
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const SCRIPT = path.join(ROOT, 'bin', 'gstack-session-update');
+const HOOK = 'hosts/claude/hooks/question-log-hook';
+const bases: string[] = [];
+afterEach(() => { for (const b of bases.splice(0)) fs.rmSync(b, { recursive: true, force: true }); });
+
+function git(cwd: string, ...args: string[]): string {
+  return execFileSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000 }).trim();
+}
+
+function makeFixture() {
+  const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-supd-hooks-'));
+  bases.push(base);
+  const origin = path.join(base, 'origin.git');
+  const seed = path.join(base, 'seed');
+  const install = path.join(base, 'install');
+  const state = path.join(base, 'state');
+  const home = path.join(base, 'home');
+  fs.mkdirSync(state, { recursive: true });
+  fs.mkdirSync(path.join(home, '.claude'), { recursive: true });
+  execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 30_000 });
+  fs.mkdirSync(path.join(seed, 'bin'), { recursive: true });
+  fs.mkdirSync(path.join(seed, path.dirname(HOOK)), { recursive: true });
+  fs.writeFileSync(path.join(seed, 'VERSION'), '1.0.0\n');
+  fs.writeFileSync(path.join(seed, 'bin', 'gstack-config'),
+    '#!/usr/bin/env bash\nif [ "$1" = "get" ]; then case "$2" in auto_upgrade) echo true;; skill_prefix) echo false;; *) echo "";; esac; fi\nexit 0\n', { mode: 0o755 });
+  fs.writeFileSync(path.join(seed, 'bin', 'gstack-patch-names'), '#!/usr/bin/env bash\nexit 0\n', { mode: 0o755 });
+  // Stub setup: records each run, and registers the hook the way real setup
+  // does, which is where the check reads the hook list from.
+  fs.writeFileSync(path.join(seed, 'setup'), [
+    '#!/usr/bin/env bash',
+    'echo "$*" >> "$SETUP_CALLS"',
+    `LOG_HOOK="$(_hook_command_path ${HOOK} || true)"`,
+    'exit 0',
+  ].join('\n') + '\n', { mode: 0o755 });
+  fs.copyFileSync(path.join(ROOT, HOOK), path.join(seed, HOOK));
+  fs.chmodSync(path.join(seed, HOOK), 0o755);
+  fs.writeFileSync(path.join(seed, `${HOOK}.ts`), "import { note } from './note';\nconsole.log(note);\n");
+  fs.writeFileSync(path.join(seed, path.dirname(HOOK), 'note.ts'), "export const note = 'old hook ran';\n");
+  git(seed, 'init', '-q');
+  git(seed, 'add', '-A');
+  git(seed, 'commit', '-q', '-m', 'seed');
+  git(seed, 'branch', '-M', 'main');
+  git(seed, 'remote', 'add', 'origin', origin);
+  git(seed, 'push', '-q', 'origin', 'main');
+  execFileSync('git', ['clone', '-q', origin, install], { timeout: 30_000 });
+  // An existing registration pointing into the live checkout.
+  const settings = path.join(home, '.claude', 'settings.json');
+  const hookCmd = path.join(install, HOOK);
+  fs.writeFileSync(settings, JSON.stringify({ hooks: { PostToolUse: [{ matcher: 'AskUserQuestion', hooks: [{ type: 'command', command: hookCmd }] }] } }, null, 2));
+  return { base, seed, install, state, home, settings, hookCmd, calls: path.join(base, 'calls') };
+}
+type Fx = ReturnType<typeof makeFixture>;
+
+function release(fx: Fx, version: string, edit: () => void) {
+  edit();
+  fs.writeFileSync(path.join(fx.seed, 'VERSION'), `${version}\n`);
+  git(fx.seed, 'add', '-A');
+  git(fx.seed, 'commit', '-qm', `release ${version}`);
+  git(fx.seed, 'push', '-q', 'origin', 'main');
+}
+
+function runHook(fx: Fx) {
+  return spawnSync('bash', [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_CALLS: fx.calls, TMPDIR: fx.base },
+    timeout: 20_000,
+  });
+}
+
+async function waitFor(fx: Fx, pattern: RegExp, ms = 15_000): Promise<string> {
+  const logFile = path.join(fx.state, 'analytics', 'session-update.log');
+  const deadline = Date.now() + ms;
+  let content = '';
+  while (Date.now() < deadline) {
+    content = fs.existsSync(logFile) ? fs.readFileSync(logFile, 'utf8') : '';
+    if (pattern.test(content) && !fs.existsSync(path.join(fx.state, '.setup-lock'))) return content;
+    await new Promise(r => setTimeout(r, 100));
+  }
+  throw new Error(`timed out waiting for ${pattern}; log:\n${content}`);
+}
+
+/** Run the hook the existing registration names, as Claude Code would. */
+function runRegisteredHook(fx: Fx) {
+  const settings = JSON.parse(fs.readFileSync(fx.settings, 'utf8'));
+  const cmd = settings.hooks.PostToolUse[0].hooks[0].command as string;
+  return spawnSync('/bin/sh', ['-c', cmd], { encoding: 'utf8', input: '{}', env: { PATH: `${path.dirname(process.execPath)}:${process.env.PATH}` }, timeout: 20_000 });
+}
+
+describe.skipIf(process.platform === 'win32')('gstack-session-update: incoming hooks are parse-checked before the fast-forward', () => {
+  test('an unparseable shim at the incoming revision holds the update; the checkout and the old hook stay', async () => {
+    const fx = makeFixture();
+    const oldHead = git(fx.install, 'rev-parse', 'HEAD');
+    const shimLines = fs.readFileSync(path.join(fx.seed, HOOK), 'utf8').split('\n').length;
+    release(fx, '1.1.0', () => fs.appendFileSync(path.join(fx.seed, HOOK), 'if then\n'));
+
+    expect(runHook(fx).stdout).toBe('');
+    const log = await waitFor(fx, /Z HELD hook-does-not-parse/);
+    expect(log).toContain(`HELD hook-does-not-parse: ${HOOK}:${shimLines}: syntax error`);
+    expect(git(fx.install, 'rev-parse', 'HEAD')).toBe(oldHead);
+    expect(fs.readFileSync(path.join(fx.install, 'VERSION'), 'utf8')).toBe('1.0.0\n');
+    expect(fs.existsSync(fx.calls)).toBe(false);
+    expect(fs.readFileSync(path.join(fx.state, 'session-update-pending'), 'utf8')).toContain(`pull=hook-does-not-parse: ${HOOK}:${shimLines}:`);
+    expect(fs.existsSync(path.join(fx.state, 'just-upgraded-from'))).toBe(false);
+    const ran = runRegisteredHook(fx);
+    expect(ran.status).toBe(0);
+    expect(ran.stdout).toContain('old hook ran');
+    // No leftover archive extraction.
+    expect(fs.readdirSync(fx.base).filter(f => f.startsWith('gstack-session-hooks-'))).toEqual([]);
+
+    // The next session start says what was held, where, and that the old hooks run.
+    const next = runHook(fx).stdout;
+    expect(next).toContain(`gstack auto-update: update held (hook-does-not-parse: ${HOOK}:${shimLines}:`);
+    expect(next).toContain('your current hooks keep running');
+    expect(next).toContain(`This is a gstack bug; report ${HOOK}:${shimLines}: `);
+    expect(next).toContain('https://github.com/garrytan/gstack/blob/main/docs/troubleshooting.md#auto-update-hook-does-not-parse');
+  });
+
+  test('a broken module the incoming TypeScript entry imports holds the update', async () => {
+    const fx = makeFixture();
+    const oldHead = git(fx.install, 'rev-parse', 'HEAD');
+    release(fx, '1.1.0', () => fs.writeFileSync(path.join(fx.seed, path.dirname(HOOK), 'note.ts'), 'export const note = ;\n'));
+    runHook(fx);
+    const log = await waitFor(fx, /Z HELD /);
+    expect(log).toContain(`HELD hook-does-not-parse: ${path.dirname(HOOK)}/note.ts:1:`);
+    expect(git(fx.install, 'rev-parse', 'HEAD')).toBe(oldHead);
+    expect(runRegisteredHook(fx).stdout).toContain('old hook ran');
+  });
+
+  test('a parseable incoming revision fast-forwards and runs setup (control)', async () => {
+    const fx = makeFixture();
+    release(fx, '1.1.0', () => fs.writeFileSync(path.join(fx.seed, path.dirname(HOOK), 'note.ts'), "export const note = 'new hook ran';\n"));
+    runHook(fx);
+    await waitFor(fx, /Z UPDATED from=1\.0\.0 to=1\.1\.0/);
+    expect(fs.readFileSync(path.join(fx.install, 'VERSION'), 'utf8')).toBe('1.1.0\n');
+    expect(runRegisteredHook(fx).stdout).toContain('new hook ran');
+  });
+
+  test('the hook check runs after the Bun floor check and before the fast-forward', () => {
+    const src = fs.readFileSync(SCRIPT, 'utf8');
+    const bun = src.indexOf('HELD=$(gstack_bun_incoming_hold');
+    const hooks = src.indexOf('HELD=$(hook_incoming_hold');
+    const ff = src.indexOf('merge --ff-only');
+    expect(bun).toBeGreaterThan(-1);
+    expect(hooks).toBeGreaterThan(bun);
+    expect(ff).toBeGreaterThan(hooks);
+  });
+});