chore(agents): address review on the session-start hook
Three findings from the review, plus a bug the first one uncovered. Sync on every session, not only when the database is missing. Cloud containers reuse a cached workspace, so "already present" does not mean "still current" — the two runs in testing came back 169 and then 168 issues, which is precisely the drift that makes a skipped sync serve stale data. Installing bd and warming the build still check first, being genuinely one-time. Making the sync unconditional exposed a worse failure than the one it fixed. `bd init` exits non-zero when the remote is unreachable but still leaves a .beads/embeddeddolt behind — an empty database. The old check tested only for that directory, so a failed sync swapped an empty tracker over a good one and logged success. Verified against a bogus remote: the good tracker was destroyed. The sync now checks bd's exit status and confirms the result actually carries issues before installing it, and the swap parks the old database in the temp directory until the move succeeds, so a failure keeps the previous tracker rather than leaving none. Re-verified: the 168-issue tracker survives a failed sync and the warning says so. An empty remote tracker is indistinguishable from a failed clone here. Keeping what we already have is the safe way to be wrong. The bd installer pipeline redirected only `bash`, so a curl failure (DNS, 403, TLS) left /tmp/bd-install.log empty — exactly when it is read. The whole pipeline is redirected now. Also ignore *.gate.lock*: bd drops .beads.gate.lock beside .beads/ on every run, which otherwise shows up as untracked noise in every session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DyHK7L8S1XDX3EihTnSKaX
This commit is contained in:
@@ -11,8 +11,13 @@
|
|||||||
# 3. The Rust workspace is uncompiled, so the first `cargo test` pays a
|
# 3. The Rust workspace is uncompiled, so the first `cargo test` pays a
|
||||||
# cold build before it reports anything.
|
# cold build before it reports anything.
|
||||||
#
|
#
|
||||||
# Idempotent: every step checks before acting. Nothing here writes to a
|
# The tracker is re-synced on every session, not just when the database is
|
||||||
# tracked file, stages anything, or creates a commit.
|
# missing: cloud containers reuse a cached workspace, so "already present"
|
||||||
|
# does not mean "still current", and a silently ageing tracker is the exact
|
||||||
|
# failure this hook exists to prevent. Installing bd and warming the build
|
||||||
|
# do check first, being genuinely one-time.
|
||||||
|
#
|
||||||
|
# Nothing here writes to a tracked file, stages anything, or creates a commit.
|
||||||
set -uo pipefail
|
set -uo pipefail
|
||||||
|
|
||||||
ROOT="${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}"
|
ROOT="${CLAUDE_PROJECT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}"
|
||||||
@@ -31,8 +36,10 @@ log() { printf '[session-start] %s\n' "$*"; }
|
|||||||
export PATH="$PATH:/root/go/bin:$HOME/go/bin"
|
export PATH="$PATH:/root/go/bin:$HOME/go/bin"
|
||||||
if ! command -v bd >/dev/null 2>&1; then
|
if ! command -v bd >/dev/null 2>&1; then
|
||||||
log "installing bd (beads)…"
|
log "installing bd (beads)…"
|
||||||
if curl -fsSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh \
|
# Redirect the whole pipeline: with only `bash` redirected, a curl failure
|
||||||
| bash >/tmp/bd-install.log 2>&1; then
|
# (DNS, 403, TLS) would leave the log empty.
|
||||||
|
if { curl -fsSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh \
|
||||||
|
| bash; } >/tmp/bd-install.log 2>&1; then
|
||||||
log "bd installed"
|
log "bd installed"
|
||||||
else
|
else
|
||||||
log "WARNING: bd install failed; see /tmp/bd-install.log. Issue tracking unavailable."
|
log "WARNING: bd install failed; see /tmp/bd-install.log. Issue tracking unavailable."
|
||||||
@@ -43,11 +50,12 @@ if [ -n "${CLAUDE_ENV_FILE:-}" ]; then
|
|||||||
fi
|
fi
|
||||||
command -v bd >/dev/null 2>&1 && bd metrics off >/dev/null 2>&1
|
command -v bd >/dev/null 2>&1 && bd metrics off >/dev/null 2>&1
|
||||||
|
|
||||||
# ── 2. Hydrate the issue tracker from the Dolt remote ──────────────────────
|
# ── 2. Sync the issue tracker from the Dolt remote ─────────────────────────
|
||||||
# `bd init` in-place rewrites .beads/config.yaml and commits, so the sync
|
# `bd init` in-place rewrites .beads/config.yaml and commits, so the sync
|
||||||
# happens in a throwaway directory and only the database is copied back.
|
# happens in a throwaway directory and only the database is moved back.
|
||||||
# The directory must be named after the database (see .beads/metadata.json).
|
# That directory must be named after the database (see .beads/metadata.json),
|
||||||
if command -v bd >/dev/null 2>&1 && [ ! -d .beads/embeddeddolt ]; then
|
# since bd derives the database name from it.
|
||||||
|
if command -v bd >/dev/null 2>&1; then
|
||||||
DB_NAME="$(jq -r '.dolt_database // "improvise"' .beads/metadata.json 2>/dev/null || echo improvise)"
|
DB_NAME="$(jq -r '.dolt_database // "improvise"' .beads/metadata.json 2>/dev/null || echo improvise)"
|
||||||
ORIGIN="$(git remote get-url origin 2>/dev/null || true)"
|
ORIGIN="$(git remote get-url origin 2>/dev/null || true)"
|
||||||
case "$ORIGIN" in
|
case "$ORIGIN" in
|
||||||
@@ -58,15 +66,47 @@ if command -v bd >/dev/null 2>&1 && [ ! -d .beads/embeddeddolt ]; then
|
|||||||
if [ -n "$ORIGIN" ]; then
|
if [ -n "$ORIGIN" ]; then
|
||||||
log "syncing issue tracker from $ORIGIN …"
|
log "syncing issue tracker from $ORIGIN …"
|
||||||
TMP="$(mktemp -d)"
|
TMP="$(mktemp -d)"
|
||||||
mkdir -p "$TMP/$DB_NAME"
|
STAGE="$TMP/$DB_NAME"
|
||||||
(
|
mkdir -p "$STAGE"
|
||||||
cd "$TMP/$DB_NAME" && git init -q . &&
|
|
||||||
|
# `bd init` exits non-zero when the remote is unreachable but STILL leaves
|
||||||
|
# a .beads/embeddeddolt behind — an empty database. Testing only for the
|
||||||
|
# directory would swap that over a good tracker, so check the status and
|
||||||
|
# confirm the result actually carries issues. An empty remote tracker is
|
||||||
|
# indistinguishable from a failed clone here; keeping what we already have
|
||||||
|
# is the safe way to be wrong.
|
||||||
|
sync_ok=0
|
||||||
|
if (
|
||||||
|
cd "$STAGE" && git init -q . &&
|
||||||
BD_NON_INTERACTIVE=1 bd init --non-interactive --skip-agents --skip-hooks -q \
|
BD_NON_INTERACTIVE=1 bd init --non-interactive --skip-agents --skip-hooks -q \
|
||||||
--remote "git+$ORIGIN"
|
--remote "git+$ORIGIN"
|
||||||
) >/tmp/bd-sync.log 2>&1
|
) >/tmp/bd-sync.log 2>&1 && [ -d "$STAGE/.beads/embeddeddolt" ]; then
|
||||||
if [ -d "$TMP/$DB_NAME/.beads/embeddeddolt" ]; then
|
listing="$( (cd "$STAGE" && bd list --all 2>/dev/null) || true )"
|
||||||
cp -rf "$TMP/$DB_NAME/.beads/embeddeddolt" .beads/embeddeddolt
|
case "$listing" in
|
||||||
log "issue tracker synced — 'bd ready' now reflects the remote, not issues.jsonl"
|
"" | "No issues found."*) ;;
|
||||||
|
*) sync_ok=1 ;;
|
||||||
|
esac
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ "$sync_ok" -eq 1 ]; then
|
||||||
|
# Swap the verified result in, parking the old database inside $TMP
|
||||||
|
# until the move succeeds — a failed install must leave the previous
|
||||||
|
# tracker in place rather than none at all. The holding spot is under
|
||||||
|
# $TMP, not .beads/, so nothing untracked ever appears in the repo.
|
||||||
|
[ -d .beads/embeddeddolt ] && mv .beads/embeddeddolt "$TMP/previous-db"
|
||||||
|
if mv "$STAGE/.beads/embeddeddolt" .beads/embeddeddolt; then
|
||||||
|
log "issue tracker synced — 'bd ready' now reflects the remote, not issues.jsonl"
|
||||||
|
elif [ -d "$TMP/previous-db" ]; then
|
||||||
|
mv "$TMP/previous-db" .beads/embeddeddolt
|
||||||
|
log "WARNING: could not install the synced tracker; kept the previous database."
|
||||||
|
log " It may be out of date — verify any bead against the code."
|
||||||
|
else
|
||||||
|
log "WARNING: could not install the synced tracker and there is no previous one."
|
||||||
|
log " .beads/issues.jsonl is a STALE export — verify any bead against the code."
|
||||||
|
fi
|
||||||
|
elif [ -d .beads/embeddeddolt ]; then
|
||||||
|
log "WARNING: tracker sync failed (see /tmp/bd-sync.log); kept the previous database."
|
||||||
|
log " It may be out of date — verify any bead against the code."
|
||||||
else
|
else
|
||||||
log "WARNING: tracker sync failed; see /tmp/bd-sync.log."
|
log "WARNING: tracker sync failed; see /tmp/bd-sync.log."
|
||||||
log " .beads/issues.jsonl is a STALE export — verify any bead against the code."
|
log " .beads/issues.jsonl is a STALE export — verify any bead against the code."
|
||||||
|
|||||||
@@ -33,3 +33,6 @@ proptest-regressions/
|
|||||||
# ...except the checked-in seed knowledge base, which agents load at session start.
|
# ...except the checked-in seed knowledge base, which agents load at session start.
|
||||||
prolog-sessions/*
|
prolog-sessions/*
|
||||||
!prolog-sessions/kb.pl
|
!prolog-sessions/kb.pl
|
||||||
|
|
||||||
|
# Beads runtime locks (bd creates these next to .beads/)
|
||||||
|
*.gate.lock*
|
||||||
|
|||||||
Reference in New Issue
Block a user