fix(hooks): make the tracker swap safe on every failure path
Review of #7 found the swap could corrupt or lose the database: the move of the live database out of the way was unchecked, so if it failed the next move put the new database *inside* the old directory and still reported success; the restore was unchecked too, and the `rm -rf "$TMP"` that followed could then delete the only remaining copy. All three are real. The root cause was doing the critical swap across filesystems, where a move is a copy that can half-finish. install_tracker() removes the class instead of patching the cases. The staged copy is brought alongside the live one first, so every step after that is a same-filesystem rename: it either happens or it does not. The previous database is removed only once the new one is in place, and every move is checked, with a loud message if a restore fails and leaves it parked. The caller collapses to one condition, which also removed a duplicated pair of warning branches. Proved rather than asserted, since this block has now been wrong twice. `mv` was stubbed to fail at each point in turn: - old-move fails -> rc=1, live database intact, no nesting, no leftovers - final rename fails -> rc=1, live database restored, no leftovers - missing source -> rc=1, live database intact A first attempt at the third case used chmod, which proves nothing as root; the stub is what actually exercises the path. The transient .incoming/.previous directories are gitignored so a crash mid-swap cannot leave untracked noise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DyHK7L8S1XDX3EihTnSKaX
This commit is contained in:
@@ -30,6 +30,41 @@ fi
|
|||||||
|
|
||||||
log() { printf '[session-start] %s\n' "$*"; }
|
log() { printf '[session-start] %s\n' "$*"; }
|
||||||
|
|
||||||
|
# Install a verified database at .beads/embeddeddolt.
|
||||||
|
#
|
||||||
|
# The staged copy lives on another filesystem, so it is brought alongside the
|
||||||
|
# live one FIRST. Every step after that is a same-filesystem rename, which
|
||||||
|
# either happens or does not — no half-finished copy. The previous database is
|
||||||
|
# removed only once the new one is in place, so no failure path leaves the repo
|
||||||
|
# without a tracker, and every move is checked: an unchecked one would let the
|
||||||
|
# new database land *inside* the old directory and still report success.
|
||||||
|
install_tracker() {
|
||||||
|
staged=$1
|
||||||
|
live=.beads/embeddeddolt
|
||||||
|
incoming=$live.incoming
|
||||||
|
previous=$live.previous
|
||||||
|
|
||||||
|
rm -rf "$incoming" "$previous" || return 1
|
||||||
|
cp -rf "$staged" "$incoming" || { rm -rf "$incoming"; return 1; }
|
||||||
|
|
||||||
|
if [ -d "$live" ] && ! mv "$live" "$previous"; then
|
||||||
|
rm -rf "$incoming"
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if mv "$incoming" "$live"; then
|
||||||
|
rm -rf "$previous"
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Could not put the new one in place; restore what was there.
|
||||||
|
if [ -d "$previous" ] && ! mv "$previous" "$live"; then
|
||||||
|
log "ERROR: tracker left at $previous — move it back to $live by hand"
|
||||||
|
fi
|
||||||
|
rm -rf "$incoming"
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
# ── 1. bd on PATH ──────────────────────────────────────────────────────────
|
# ── 1. bd on PATH ──────────────────────────────────────────────────────────
|
||||||
# The embedded Dolt engine needs CGO, so this builds from source (~3 min).
|
# The embedded Dolt engine needs CGO, so this builds from source (~3 min).
|
||||||
# The container image is cached after the hook, so it is paid once.
|
# The container image is cached after the hook, so it is paid once.
|
||||||
@@ -91,22 +126,8 @@ if command -v bd >/dev/null 2>&1; then
|
|||||||
esac
|
esac
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [ "$sync_ok" -eq 1 ]; then
|
if [ "$sync_ok" -eq 1 ] && install_tracker "$STAGE/.beads/embeddeddolt"; then
|
||||||
# Swap the verified result in, parking the old database inside $TMP
|
log "issue tracker synced — 'bd ready' now reflects the remote, not issues.jsonl"
|
||||||
# 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
|
elif [ -d .beads/embeddeddolt ]; then
|
||||||
log "WARNING: tracker sync failed (see /tmp/bd-sync.log); kept the previous database."
|
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."
|
log " It may be out of date — verify any bead against the code."
|
||||||
|
|||||||
@@ -36,3 +36,5 @@ prolog-sessions/*
|
|||||||
|
|
||||||
# Beads runtime locks (bd creates these next to .beads/)
|
# Beads runtime locks (bd creates these next to .beads/)
|
||||||
*.gate.lock*
|
*.gate.lock*
|
||||||
|
.beads/embeddeddolt.incoming/
|
||||||
|
.beads/embeddeddolt.previous/
|
||||||
|
|||||||
Reference in New Issue
Block a user