From 40e4a53f74257d8b7b1e8cf09da5581222452d86 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 10:32:19 -0700 Subject: [PATCH] =?UTF-8?q?fix(gbrain):=20brain=20worktree=20advances=20on?= =?UTF-8?q?=20the=20daily=20sync=20=E2=80=94=20no=20more=20silently=20stal?= =?UTF-8?q?e=20brains=20(#2516)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daily pull refreshed only ~/.gstack itself, never the detached worktree at ~/.gstack-brain-worktree that gbrain actually indexes — so after setup the brain served stale pages forever unless setup-gbrain/sync-gbrain happened to run. brain-sync --once now advances the worktree once per 24h behind an ATTEMPT stamp (.brain-worktree-last-advance — a persistently-failing advance warns once a day, not at every skill boundary), inside the existing run lock and before any ingest step touches the worktree. The new gstack-gbrain-source-wireup --advance-only is built for the unattended cadence: git-only (no gbrain prereqs), pins every operation to the managed worktree (refuses paths that are not worktrees of the artifacts repo), refuses dirty worktrees, and never runs the force-remove recovery — a cron path must not be able to delete local changes. A static pin keeps the force-remove out. docs/gbrain-sync.md stops overclaiming the old cadence. Fixes #2516. Co-Authored-By: Claude Fable 5 --- bin/gstack-brain-sync | 23 +++ bin/gstack-gbrain-source-wireup | 53 ++++++- docs/gbrain-sync.md | 8 + test/gbrain-source-worktree-advance.test.ts | 157 ++++++++++++++++++++ 4 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 test/gbrain-source-worktree-advance.test.ts diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index b3f460377..1a9c7b5ff 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -377,6 +377,29 @@ subcmd_once() { local mode mode=$("$CONFIG_BIN" get artifacts_sync_mode 2>/dev/null || echo off) + # #2516: advance the brain worktree gbrain indexes to the artifacts repo's + # HEAD once a day — previously it only moved when setup-gbrain / sync-gbrain + # / brain-restore ran, so brains silently served stale code forever. Runs + # inside THIS run lock (never concurrent with the ingest steps below) and + # before they touch the worktree. Attempt-throttled: the stamp is written on + # ATTEMPT, so a persistently-failing advance warns once per 24h, not at + # every skill boundary. The advance itself refuses dirty or unmanaged + # worktrees and never force-removes (see gstack-gbrain-source-wireup). + if [ -e "${GSTACK_BRAIN_WORKTREE:-$HOME/.gstack-brain-worktree}" ]; then + local adv_stamp adv_now adv_last adv_age + adv_stamp="$GSTACK_HOME/.brain-worktree-last-advance" + adv_now=$(date +%s) + adv_last=$(cat "$adv_stamp" 2>/dev/null || echo 0) + case "$adv_last" in ''|*[!0-9]*) adv_last=0 ;; esac + adv_age=$(( adv_now - adv_last )) + if [ "$adv_age" -ge 86400 ]; then + echo "$adv_now" > "$adv_stamp" 2>/dev/null || true + if ! "$SCRIPT_DIR/gstack-gbrain-source-wireup" --advance-only 1>&2; then + echo "BRAIN_SYNC: warning: brain worktree advance failed — gbrain may be indexing stale code (run gstack-gbrain-source-wireup to repair)" >&2 + fi + fi + fi + # #2549 unpushed-commit detector: a prior drain may have COMMITTED but # failed to push (auth blip, offline). The data was never lost — it sits in # a local commit — but nothing re-pushed it until NEW changes arrived. diff --git a/bin/gstack-gbrain-source-wireup b/bin/gstack-gbrain-source-wireup index a8bf7e42d..a3c20b9c6 100755 --- a/bin/gstack-gbrain-source-wireup +++ b/bin/gstack-gbrain-source-wireup @@ -12,6 +12,7 @@ # gstack-gbrain-source-wireup --uninstall [--source-id ] # [--database-url ] # gstack-gbrain-source-wireup --probe +# gstack-gbrain-source-wireup --advance-only # daily unattended worktree advance (#2516) # gstack-gbrain-source-wireup --help # # Exit codes: @@ -64,6 +65,7 @@ while [ $# -gt 0 ]; do case "$1" in --uninstall) MODE="uninstall"; shift ;; --probe) MODE="probe"; shift ;; + --advance-only) MODE="advance-only"; shift ;; --strict) STRICT=1; shift ;; --no-pull) NO_PULL=1; shift ;; --source-id) SOURCE_ID="$2"; shift 2 ;; @@ -336,6 +338,50 @@ do_wireup() { echo "pages_synced=$(echo "$sync_out" | grep -oE '[0-9]+ pages? imported' | head -1 || echo 'incremental')" } +do_advance_only() { + # Daily unattended advance (#2516): the brain worktree gbrain indexes only + # moved when setup-gbrain / sync-gbrain / brain-restore ran, so brains + # silently served stale code. This mode is git-only (no gbrain prereqs) and + # SAFE for a cron cadence: it refuses dirty worktrees and NEVER runs + # ensure_worktree's force-remove recovery — an unattended path must not be + # able to delete local worktree changes. All git ops are pinned to + # $GSTACK_HOME / $WORKTREE, never cwd-derived. + [ -d "$GSTACK_HOME/.git" ] || { warn "advance-only: no artifacts repo at $GSTACK_HOME; nothing to advance"; exit 0; } + if [ ! -d "$WORKTREE/.git" ] && [ ! -f "$WORKTREE/.git" ]; then + warn "advance-only: no managed worktree at $WORKTREE (run the setup-gbrain wireup first)" + exit 0 + fi + # Managed-marker check: refuse anything that is not a worktree OF the + # artifacts repo — a misconfigured GSTACK_BRAIN_WORKTREE pointing at a user + # repo must never be advanced/detached. + local gitdir home_git + gitdir=$(git -C "$WORKTREE" rev-parse --absolute-git-dir 2>/dev/null || echo "") + # Physical path for the comparison: rev-parse returns resolved paths, while + # $GSTACK_HOME may reach the same place through a symlink (macOS /var/folders). + home_git=$(cd "$GSTACK_HOME/.git" 2>/dev/null && pwd -P || echo "$GSTACK_HOME/.git") + case "$gitdir" in + "$home_git/worktrees/"*) : ;; + *) warn "advance-only: $WORKTREE is not a worktree of $GSTACK_HOME (gitdir: ${gitdir:-unreadable}); refusing"; exit 0 ;; + esac + if [ -n "$(git -C "$WORKTREE" status --porcelain 2>/dev/null)" ]; then + warn "advance-only: worktree at $WORKTREE has local changes; refusing to advance them away" + exit 0 + fi + local sha cur + sha=$(git -C "$GSTACK_HOME" rev-parse HEAD 2>/dev/null) || { warn "advance-only: cannot read parent HEAD"; exit 0; } + cur=$(git -C "$WORKTREE" rev-parse HEAD 2>/dev/null || echo "") + if [ "$cur" = "$sha" ]; then + echo "advance-only: up-to-date at $sha" + return 0 + fi + if ( cd "$WORKTREE" && git checkout --detach "$sha" 2>&1 | prefix; exit "${PIPESTATUS[0]}" ); then + echo "advance-only: advanced $WORKTREE to $sha" + else + warn "advance-only: could not advance $WORKTREE to $sha; NOT force-resetting on the unattended path. Run gstack-gbrain-source-wireup to repair." + exit 1 + fi +} + do_uninstall() { local id id=$(derive_source_id) || die "cannot derive source id; pass --source-id explicitly" 3 @@ -356,7 +402,8 @@ do_uninstall() { } case "$MODE" in - probe) do_probe ;; - wireup) do_wireup ;; - uninstall) do_uninstall ;; + probe) do_probe ;; + wireup) do_wireup ;; + uninstall) do_uninstall ;; + advance-only) do_advance_only ;; esac diff --git a/docs/gbrain-sync.md b/docs/gbrain-sync.md index e6c0466e4..330ed29e2 100644 --- a/docs/gbrain-sync.md +++ b/docs/gbrain-sync.md @@ -166,6 +166,14 @@ The preamble runs `git fetch` + `git merge --ff-only` once per 24 hours (cached via `~/.gstack/.brain-last-pull`). You don't need to think about this — it happens automatically at the first skill invocation each day. +Historical note (#2516): that daily pull refreshed only `~/.gstack` itself — +NOT the detached worktree at `~/.gstack-brain-worktree` that gbrain actually +indexes, so the brain silently served stale pages until the next +setup-gbrain/sync-gbrain run. Since this fix, the daily sync also advances +the brain worktree (`gstack-gbrain-source-wireup --advance-only`, throttled +via `~/.gstack/.brain-worktree-last-advance`); a failed advance warns instead +of failing silently, and never force-resets a dirty worktree. + ## Uninstall ```bash diff --git a/test/gbrain-source-worktree-advance.test.ts b/test/gbrain-source-worktree-advance.test.ts new file mode 100644 index 000000000..3763bcf57 --- /dev/null +++ b/test/gbrain-source-worktree-advance.test.ts @@ -0,0 +1,157 @@ +/** + * #2516: the brain worktree gbrain indexes must advance on the daily sync — + * and the unattended advance path must be SAFE: refuse dirty worktrees, + * refuse anything that is not a worktree of the artifacts repo, and never + * force-remove. (Pre-fix, the worktree only moved when setup-gbrain / + * sync-gbrain / brain-restore ran, so brains silently served stale pages.) + */ +import { describe, test as _test, expect, beforeEach, afterEach } from 'bun:test'; +const test = (name: string, fn: any) => _test(name, fn, 30000); +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { spawnSync } from 'child_process'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BIN = path.join(ROOT, 'bin'); + +let tmpHome: string; + +function run(argv: string[], env: Record = {}) { + const full = path.join(BIN, argv[0]); + const res = spawnSync(full, argv.slice(1), { + env: { ...process.env, HOME: tmpHome, GSTACK_HOME: tmpHome, ...env }, + encoding: 'utf-8', + cwd: ROOT, + }); + return { stdout: res.stdout || '', stderr: res.stderr || '', status: res.status ?? -1 }; +} + +function git(args: string[], cwd: string) { + const res = spawnSync('git', args, { cwd, encoding: 'utf-8' }); + return { stdout: (res.stdout || '').trim(), status: res.status ?? -1 }; +} + +function commit(cwd: string, msg: string): string { + fs.appendFileSync(path.join(cwd, 'artifact.md'), `${msg}\n`); + git(['add', 'artifact.md'], cwd); + git(['commit', '-q', '-m', msg], cwd); + return git(['rev-parse', 'HEAD'], cwd).stdout; +} + +const worktreePath = () => path.join(tmpHome, '.gstack-brain-worktree'); + +function makeArtifactsRepoWithWorktree(): { head: string } { + git(['init', '-q', '-b', 'main'], tmpHome); + git(['config', 'user.email', 't@t'], tmpHome); + git(['config', 'user.name', 't'], tmpHome); + const head = commit(tmpHome, 'seed'); + git(['worktree', 'add', '--detach', worktreePath(), head], tmpHome); + return { head }; +} + +beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'wtree-adv-home-')); +}); + +afterEach(() => { + fs.rmSync(tmpHome, { recursive: true, force: true }); +}); + +describe('gstack-gbrain-source-wireup --advance-only (#2516)', () => { + test('advances a clean, behind worktree to the parent HEAD', () => { + makeArtifactsRepoWithWorktree(); + const newHead = commit(tmpHome, 'second'); + const r = run(['gstack-gbrain-source-wireup', '--advance-only']); + expect(r.status).toBe(0); + expect(r.stdout + r.stderr).toContain('advanced'); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(newHead); + }); + + test('up-to-date worktree is a no-op', () => { + const { head } = makeArtifactsRepoWithWorktree(); + const r = run(['gstack-gbrain-source-wireup', '--advance-only']); + expect(r.status).toBe(0); + expect(r.stdout + r.stderr).toContain('up-to-date'); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(head); + }); + + test('REFUSES a dirty worktree — local changes are never advanced away', () => { + const { head } = makeArtifactsRepoWithWorktree(); + commit(tmpHome, 'second'); + fs.writeFileSync(path.join(worktreePath(), 'artifact.md'), 'local edit\n'); + const r = run(['gstack-gbrain-source-wireup', '--advance-only']); + expect(r.status).toBe(0); // benign skip, not a hard failure + expect(r.stderr).toContain('local changes'); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(head); // untouched + expect(fs.readFileSync(path.join(worktreePath(), 'artifact.md'), 'utf-8')).toBe('local edit\n'); + }); + + test('REFUSES a path that is not a worktree of the artifacts repo', () => { + makeArtifactsRepoWithWorktree(); + // A standalone user repo masquerading as the brain worktree. + const userRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'wtree-adv-user-')); + try { + git(['init', '-q', '-b', 'main'], userRepo); + git(['config', 'user.email', 't@t'], userRepo); + git(['config', 'user.name', 't'], userRepo); + const userHead = commit(userRepo, 'user work'); + const r = run(['gstack-gbrain-source-wireup', '--advance-only'], { + GSTACK_BRAIN_WORKTREE: userRepo, + }); + expect(r.status).toBe(0); + expect(r.stderr).toContain('not a worktree of'); + expect(git(['rev-parse', 'HEAD'], userRepo).stdout).toBe(userHead); // untouched + } finally { + fs.rmSync(userRepo, { recursive: true, force: true }); + } + }); + + test('missing worktree is a benign skip', () => { + git(['init', '-q', '-b', 'main'], tmpHome); + git(['config', 'user.email', 't@t'], tmpHome); + git(['config', 'user.name', 't'], tmpHome); + commit(tmpHome, 'seed'); + const r = run(['gstack-gbrain-source-wireup', '--advance-only']); + expect(r.status).toBe(0); + expect(r.stderr).toContain('no managed worktree'); + }); + + test('never contains a force-remove on the advance-only path (static pin)', () => { + // The unattended path must not be able to delete local worktree changes: + // do_advance_only may not call safe_rm_worktree, `worktree remove`, or rm -rf. + const src = fs.readFileSync(path.join(BIN, 'gstack-gbrain-source-wireup'), 'utf-8'); + const fn = src.slice(src.indexOf('do_advance_only()'), src.indexOf('do_uninstall()')); + expect(fn.length).toBeGreaterThan(100); + expect(fn).not.toContain('safe_rm_worktree'); + expect(fn).not.toContain('worktree remove'); + expect(fn).not.toContain('rm -rf'); + }); +}); + +describe('brain-sync --once daily advance wiring (#2516)', () => { + test('once advances the worktree behind a 24h attempt stamp', () => { + makeArtifactsRepoWithWorktree(); + run(['gstack-config', 'set', 'artifacts_sync_mode', 'artifacts-only']); + const second = commit(tmpHome, 'second'); + + const r1 = run(['gstack-brain-sync', '--once']); + expect(r1.status).toBe(0); + const stamp = path.join(tmpHome, '.brain-worktree-last-advance'); + expect(fs.existsSync(stamp)).toBe(true); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(second); + + // Within the 24h window: parent advances again, --once does NOT re-advance. + const third = commit(tmpHome, 'third'); + const r2 = run(['gstack-brain-sync', '--once']); + expect(r2.status).toBe(0); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(second); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).not.toBe(third); + + // Expire the stamp → the next --once advances again. + fs.writeFileSync(stamp, String(Math.floor(Date.now() / 1000) - 90000)); + const r3 = run(['gstack-brain-sync', '--once']); + expect(r3.status).toBe(0); + expect(git(['rev-parse', 'HEAD'], worktreePath()).stdout).toBe(third); + }); +});