From 031bb3689ef744af4e3965cb6d081bd776b45f4c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 19:03:23 -0700 Subject: [PATCH] =?UTF-8?q?fix(session-update):=20un-wedge=20auto-upgrade?= =?UTF-8?q?=20=E2=80=94=20autostash=20over=20local=20patches,=20log=20the?= =?UTF-8?q?=20pull's=20real=20reason?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a normal install the tracked files ARE locally patched (skill-prefix name rewrites, gbrain-refresh blocks), so the bare `git pull --ff-only` refused on every run and auto-upgrade froze forever — observed as 308 consecutive PULL_FAILED entries with the reason discarded by 2>/dev/null. Pull now runs --autostash (local patches ride over the update and pop back), stderr is captured into the log so a genuine failure names its cause, an autostash pop conflict recovers to a clean tree and re-renders the patches (gstack-patch-names + gbrain-refresh, both idempotent), and a successful pull re-renders them as a self-heal. Behavioral tests cover the wedge shape and the reason logging. Fixes #2566. Co-Authored-By: Claude Fable 5 --- bin/gstack-session-update | 32 ++++++- test/session-update-autostash.test.ts | 124 ++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 test/session-update-autostash.test.ts diff --git a/bin/gstack-session-update b/bin/gstack-session-update index 7e21e5df0..4207d84db 100755 --- a/bin/gstack-session-update +++ b/bin/gstack-session-update @@ -82,8 +82,15 @@ fi OLD_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) UPDATE_URL=$(git -C "$GSTACK_DIR" remote get-url origin 2>/dev/null || echo "") UPDATE_HOST="${UPDATE_URL#*://}"; UPDATE_HOST="${UPDATE_HOST#*@}"; UPDATE_HOST="${UPDATE_HOST%%[/:]*}" + # --autostash: locally-patched TRACKED files are the NORM on installs, not + # the exception — skill-prefix mode rewrites frontmatter names and + # `gstack-config gbrain-refresh` renders brain blocks into SKILL.md. A bare + # --ff-only refuses over those edits, so auto-upgrade wedged permanently + # (observed: 308 consecutive PULL_FAILED with the reason discarded, #2566). + # Capture stderr: the log must carry WHY a pull failed, never just the code. + PULL_ERR_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-session-pull-XXXXXX" 2>/dev/null || echo "") GSTACK_HOME="$STATE_DIR" _receipted_git open session-update "${UPDATE_HOST:-unknown}" gstack-self-update-pull "auto_upgrade=true" \ - bash -c 'git -C "$1" pull --ff-only -q 2>/dev/null' _ "$GSTACK_DIR" + bash -c 'git -C "$1" pull --ff-only --autostash -q 2>"${2:-/dev/null}"' _ "$GSTACK_DIR" "$PULL_ERR_FILE" PULL_EXIT=$? NEW_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) @@ -91,9 +98,30 @@ fi date +%s > "$THROTTLE_FILE" 2>/dev/null if [ "$PULL_EXIT" -ne 0 ]; then - log_entry "PULL_FAILED exit=$PULL_EXIT" + PULL_REASON=$(head -c 300 "$PULL_ERR_FILE" 2>/dev/null | tr '\n' ' ' | tr -s ' ') + log_entry "PULL_FAILED exit=$PULL_EXIT reason=${PULL_REASON:-unknown}" + # Autostash pop conflict leaves the stash behind and the tree half-merged. + # The local patches are REGENERABLE (prefix renames, gbrain blocks), so + # recover to a clean upstream tree and re-render them below rather than + # leaving conflict markers in a live install. + if grep -qi "autostash" "$PULL_ERR_FILE" 2>/dev/null; then + git -C "$GSTACK_DIR" checkout -q -- . 2>/dev/null + git -C "$GSTACK_DIR" stash drop -q 2>/dev/null + log_entry "AUTOSTASH_CONFLICT_RECOVERED tree_reset=1" + _PREFIX_CFG=$("$GSTACK_DIR/bin/gstack-config" get skill_prefix 2>/dev/null || echo false) + "$GSTACK_DIR/bin/gstack-patch-names" "$GSTACK_DIR" "$_PREFIX_CFG" >/dev/null 2>&1 || true + "$GSTACK_DIR/bin/gstack-config" gbrain-refresh >/dev/null 2>&1 || true + fi + rm -f "$PULL_ERR_FILE" 2>/dev/null exit 0 fi + rm -f "$PULL_ERR_FILE" 2>/dev/null + # Re-render local patches over the fresh tree (both tools are idempotent + # no-ops when the feature is unconfigured); the autostash pop usually + # preserves them, but a clean re-render costs nothing and self-heals. + _PREFIX_CFG=$("$GSTACK_DIR/bin/gstack-config" get skill_prefix 2>/dev/null || echo false) + "$GSTACK_DIR/bin/gstack-patch-names" "$GSTACK_DIR" "$_PREFIX_CFG" >/dev/null 2>&1 || true + "$GSTACK_DIR/bin/gstack-config" gbrain-refresh >/dev/null 2>&1 || true # ── If HEAD moved, run setup -q ── if [ "$OLD_HEAD" != "$NEW_HEAD" ]; then diff --git a/test/session-update-autostash.test.ts b/test/session-update-autostash.test.ts new file mode 100644 index 000000000..bf54f3a54 --- /dev/null +++ b/test/session-update-autostash.test.ts @@ -0,0 +1,124 @@ +import { describe, test, expect } from 'bun:test'; +import { execFileSync, spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +// #2566: on a normal install, tracked files are locally patched (skill-prefix +// name rewrites, gbrain-refresh blocks), so a bare `git pull --ff-only` +// refused FOREVER — 308 consecutive PULL_FAILED entries observed, with the +// reason discarded by 2>/dev/null. The fix: --autostash un-wedges the pull +// over local edits, and stderr is captured into the log so a real failure +// names its cause. + +const ROOT = path.resolve(import.meta.dir, '..'); +const SCRIPT = path.join(ROOT, 'bin', 'gstack-session-update'); + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { cwd, encoding: 'utf8' }).trim(); +} + +function makeFixture() { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-supd-')); + 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'); + fs.mkdirSync(state, { recursive: true }); + execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin]); + + fs.mkdirSync(path.join(seed, 'bin'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'VERSION'), '1.0.0\n'); + fs.writeFileSync(path.join(seed, 'SKILL.md'), '# top\nname: qa\nbody line\n'); + // Stub config: auto_upgrade on, prefix off; gbrain-refresh no-op. + 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, + }); + 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]); + return { base, origin, seed, install, state }; +} + +function runScript(install: string, state: string) { + return spawnSync('bash', [SCRIPT], { + encoding: 'utf8', + env: { ...process.env, GSTACK_DIR: install, GSTACK_STATE_DIR: state }, + timeout: 20000, + }); +} + +async function waitForLog(state: string, pattern: RegExp, ms = 15000): Promise { + const logFile = path.join(state, 'analytics', 'session-update.log'); + const deadline = Date.now() + ms; + while (Date.now() < deadline) { + const content = fs.existsSync(logFile) ? fs.readFileSync(logFile, 'utf8') : ''; + if (pattern.test(content)) return content; + await new Promise((r) => setTimeout(r, 200)); + } + return fs.existsSync(logFile) ? fs.readFileSync(logFile, 'utf8') : ''; +} + +describe('gstack-session-update pull wedge (#2566)', () => { + test('locally-patched tracked files no longer wedge the ff-only pull', async () => { + const { base, seed, install, state } = makeFixture(); + try { + // Upstream advances (edit at the TOP of SKILL.md)… + fs.writeFileSync( + path.join(seed, 'SKILL.md'), + '# top v2\nname: qa\nbody line\n', + ); + git(seed, 'commit', '-aqm', 'upstream change'); + git(seed, 'push', '-q', 'origin', 'main'); + const upstreamHead = git(seed, 'rev-parse', 'HEAD'); + + // …while the install carries a local patch at the BOTTOM (the + // prefix-rename / gbrain-block shape: tracked file, modified). + fs.appendFileSync(path.join(install, 'SKILL.md'), 'locally patched line\n'); + + const r = runScript(install, state); + expect(r.status).toBe(0); + const log = await waitForLog(state, /UPDATING|UP_TO_DATE|PULL_FAILED/); + expect(log).not.toContain('PULL_FAILED'); + expect(log).toContain('UPDATING'); + expect(git(install, 'rev-parse', 'HEAD')).toBe(upstreamHead); + // The autostash pop preserved the local patch over the new tree. + expect(fs.readFileSync(path.join(install, 'SKILL.md'), 'utf8')).toContain( + 'locally patched line', + ); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }, 30000); + + test('a genuinely failing pull logs its REASON, not just an exit code', async () => { + const { base, seed, install, state } = makeFixture(); + try { + // Diverge: local commit the remote doesn't have + remote advance → non-ff. + fs.appendFileSync(path.join(install, 'VERSION'), 'local\n'); + git(install, 'commit', '-aqm', 'local divergence'); + fs.appendFileSync(path.join(seed, 'VERSION'), 'remote\n'); + git(seed, 'commit', '-aqm', 'remote divergence'); + git(seed, 'push', '-q', 'origin', 'main'); + + const r = runScript(install, state); + expect(r.status).toBe(0); + const log = await waitForLog(state, /PULL_FAILED/); + expect(log).toContain('PULL_FAILED'); + const line = log.split('\n').find((l) => l.includes('PULL_FAILED')) ?? ''; + expect(line).toContain('reason='); + expect(line).not.toContain('reason=unknown'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }, 30000); +});