mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
fix(gbrain): brain worktree advances on the daily sync — no more silently stale brains (#2516)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
c4d91507dd
commit
40e4a53f74
@@ -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.
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
# gstack-gbrain-source-wireup --uninstall [--source-id <id>]
|
||||
# [--database-url <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 <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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, string> = {}) {
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user