-
Notifications
You must be signed in to change notification settings - Fork 0
fix(memory): backup before destructive consolidate #2435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # Takes a timestamped copy of a durable-memory file (or the whole store) BEFORE | ||
| # a destructive consolidate/rewrite, so a trim cannot erase unrecoverable | ||
| # history. The memory store is multi-writer, un-versioned, and outside git — | ||
| # without a backup, a size-threshold consolidation is a one-way delete | ||
| # (monorepo#2304: ~48KB of learnings lost on 2026-07-20 with no restore path). | ||
| # | ||
| # This script ONLY copies. It never edits, truncates, or consolidates the | ||
| # source — the agent still decides what to keep after the backup lands. | ||
| # | ||
| # Usage: | ||
| # memory-backup.sh [--backup-dir <dir>] <file> | ||
| # memory-backup.sh --all [--backup-dir <dir>] <memory-dir> | ||
| # | ||
| # Default backup root is <parent>/.memory-backups/ (sibling of the file, or | ||
| # inside the memory dir for --all). Nested under the store so it stays with | ||
| # the runtime that owns the memory; hygiene ignores nested dirs (maxdepth 1). | ||
| # | ||
| # Single-file layout: .memory-backups/<basename>.<UTC-timestamp> | ||
| # Whole-store layout: .memory-backups/store.<UTC-timestamp>/<basename> | ||
| # | ||
| # Exit codes: | ||
| # 0 backup written; path + restore command printed on stdout | ||
| # 2 usage error, missing source, or copy failure | ||
| set -Eeuo pipefail | ||
|
|
||
| mode="file" | ||
| backup_dir="" | ||
| target="" | ||
|
|
||
| usage() { | ||
| sed -n '/^# Usage:/,/^# Exit codes:/p' "$0" | sed '$d' | sed 's/^# \{0,1\}//' | ||
| } | ||
|
|
||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --all) mode="all"; shift ;; | ||
| --backup-dir) backup_dir="${2-}"; shift 2 || exit 2 ;; | ||
| -h|--help) usage; exit 0 ;; | ||
| --) shift; break ;; | ||
| -*) | ||
| echo "memory-backup: unknown argument '$1'" >&2 | ||
| usage >&2 | ||
| exit 2 | ||
| ;; | ||
| *) | ||
| if [[ -n "$target" ]]; then | ||
| echo "memory-backup: unexpected extra argument '$1'" >&2 | ||
| usage >&2 | ||
| exit 2 | ||
| fi | ||
| target="$1" | ||
| shift | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| if [[ -z "$target" ]]; then | ||
| echo "memory-backup: a file (or memory-dir with --all) is required" >&2 | ||
| usage >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| # Tests pin the timestamp so golden paths stay deterministic; production uses UTC now. | ||
| ts="${MEMORY_BACKUP_TS:-$(date -u +%Y%m%dT%H%M%SZ)}" | ||
| if ! [[ "$ts" =~ ^[0-9]{8}T[0-9]{6}Z$ ]]; then | ||
| echo "memory-backup: MEMORY_BACKUP_TS must look like YYYYMMDDTHHMMSSZ (got '$ts')" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| # Copy via a temp name in the destination dir, then rename — so a crash mid-copy | ||
| # never leaves a partial file that looks like a successful backup. | ||
| atomic_cp() { | ||
| local src="$1" dest="$2" | ||
| local dest_dir tmp | ||
| dest_dir="$(dirname "$dest")" | ||
| mkdir -p "$dest_dir" | ||
| tmp="$(mktemp "$dest_dir/.memory-backup.XXXXXX")" | ||
| # mktemp creates an empty file; replace it with the source contents. | ||
| cp -p "$src" "$tmp" | ||
| mv -f "$tmp" "$dest" | ||
| } | ||
|
|
||
| if [[ "$mode" == "file" ]]; then | ||
| if [[ ! -f "$target" ]]; then | ||
| echo "memory-backup: not a readable file: $target" >&2 | ||
| exit 2 | ||
| fi | ||
| parent="$(cd "$(dirname "$target")" && pwd)" | ||
| base="$(basename "$target")" | ||
| if [[ -z "$backup_dir" ]]; then | ||
| backup_dir="$parent/.memory-backups" | ||
| fi | ||
| dest="$backup_dir/${base}.${ts}" | ||
| if [[ -e "$dest" ]]; then | ||
| echo "memory-backup: refusing to overwrite existing backup: $dest" >&2 | ||
| exit 2 | ||
| fi | ||
| atomic_cp "$target" "$dest" | ||
| printf 'Backed up %s -> %s\n' "$target" "$dest" | ||
| printf 'Restore: cp %q %q\n' "$dest" "$target" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # --all: snapshot every top-level *.md in the memory dir (including archives — | ||
| # a recovery snapshot should be complete, not budget-filtered). | ||
| if [[ ! -d "$target" ]]; then | ||
| echo "memory-backup: not a directory: $target" >&2 | ||
| exit 2 | ||
| fi | ||
| memory_dir="$(cd "$target" && pwd)" | ||
| if [[ -z "$backup_dir" ]]; then | ||
| backup_dir="$memory_dir/.memory-backups" | ||
| fi | ||
| store_dest="$backup_dir/store.${ts}" | ||
| if [[ -e "$store_dest" ]]; then | ||
| echo "memory-backup: refusing to overwrite existing snapshot: $store_dest" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| if ! file_list="$(find "$memory_dir" -maxdepth 1 -type f -name '*.md' 2>/dev/null | sort)"; then | ||
| echo "memory-backup: failed to enumerate memory files in $memory_dir" >&2 | ||
| exit 2 | ||
| fi | ||
| if [[ -z "$file_list" ]]; then | ||
| echo "memory-backup: no top-level *.md files in $memory_dir" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| mkdir -p "$store_dest" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a later source disappears after enumeration or any copy fails, this final-named snapshot directory and its already-copied files remain behind even though the command fails. Deleting a later file during a large AGENTS.md reference: AGENTS.md:L2693-L2697 Useful? React with 👍 / 👎. |
||
| count=0 | ||
| while IFS= read -r file; do | ||
| [[ -n "$file" ]] || continue | ||
| base="$(basename "$file")" | ||
| atomic_cp "$file" "$store_dest/$base" | ||
| count=$(( count + 1 )) | ||
| done <<< "$file_list" | ||
|
|
||
| printf 'Backed up %s file(s) from %s -> %s\n' "$count" "$memory_dir" "$store_dest" | ||
| printf 'Restore one file: cp %q/<basename> %q/<basename>\n' "$store_dest" "$memory_dir" | ||
| exit 0 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # Self-test for memory-backup.sh — proves a destructive-edit precursor actually | ||
| # lands a recoverable copy (monorepo#2304), that --all snapshots the whole | ||
| # top-level store, that an existing backup is never overwritten, and that the | ||
| # source file is left byte-identical. | ||
| # | ||
| # Fixtures are throwaway files in a temp dir — no real memory touched. | ||
| set -Eeuo pipefail | ||
|
|
||
| script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| tool="$script_dir/memory-backup.sh" | ||
|
|
||
| tmp="$(mktemp -d)" | ||
| trap 'rm -rf "$tmp"' EXIT | ||
|
|
||
| failures=0 | ||
| pass() { printf 'ok — %s\n' "$1"; } | ||
| fail() { printf 'FAIL — %s\n' "$1"; failures=$(( failures + 1 )); } | ||
|
|
||
| check() { | ||
| local desc="$1" expected="$2" actual="$3" | ||
| if [[ "$expected" == "$actual" ]]; then pass "$desc"; else | ||
| fail "$desc (expected '$expected', got '$actual')" | ||
| fi | ||
| } | ||
|
|
||
| # Capture exit code without tripping set -e on an expected failure. | ||
| run() { local rc=0; "$tool" "$@" >/dev/null 2>&1 || rc=$?; echo "$rc"; } | ||
|
|
||
| export MEMORY_BACKUP_TS=20260724T003000Z | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Single-file backup lands under .memory-backups/ with the pinned timestamp, | ||
| # prints a restore command, and leaves the source untouched. | ||
| # --------------------------------------------------------------------------- | ||
| store="$tmp/single" | ||
| mkdir -p "$store" | ||
| printf 'keep-me learnings theme\n' > "$store/learnings.md" | ||
| before="$(shasum "$store/learnings.md" | awk '{print $1}')" | ||
| rc=0 | ||
| out="$("$tool" "$store/learnings.md" 2>&1)" || rc=$? | ||
| check "single-file backup exits 0" "0" "$rc" | ||
|
|
||
| dest="$store/.memory-backups/learnings.md.${MEMORY_BACKUP_TS}" | ||
| if [[ -f "$dest" ]]; then | ||
| pass "timestamped backup file exists" | ||
| else | ||
| fail "timestamped backup file exists (missing $dest)" | ||
| fi | ||
| check "backup content matches source" "$(cat "$store/learnings.md")" "$(cat "$dest")" | ||
| check "source unchanged after backup" "$before" "$(shasum "$store/learnings.md" | awk '{print $1}')" | ||
| if grep -q "Restore: cp" <<<"$out"; then | ||
| pass "stdout carries a restore command" | ||
| else | ||
| fail "stdout carries a restore command (got: $out)" | ||
| fi | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # A second backup at the same timestamp must REFUSE to overwrite (fail closed). | ||
| # --------------------------------------------------------------------------- | ||
| check "refuses to overwrite existing backup" "2" "$(run "$store/learnings.md")" | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # --all snapshots every top-level *.md, including archives. | ||
| # --------------------------------------------------------------------------- | ||
| store="$tmp/all" | ||
| mkdir -p "$store" | ||
| printf 'a\n' > "$store/MEMORY.md" | ||
| printf 'b\n' > "$store/learnings.md" | ||
| printf 'c\n' > "$store/learnings-archive-2026-07-12.md" | ||
| export MEMORY_BACKUP_TS=20260724T003001Z | ||
| rc=0 | ||
| out="$("$tool" --all "$store" 2>&1)" || rc=$? | ||
| check "--all exits 0" "0" "$rc" | ||
| snap="$store/.memory-backups/store.${MEMORY_BACKUP_TS}" | ||
| if [[ -d "$snap" ]]; then | ||
| pass "whole-store snapshot directory exists" | ||
| else | ||
| fail "whole-store snapshot directory exists" | ||
| fi | ||
| check "snapshot includes MEMORY.md" "a" "$(cat "$snap/MEMORY.md")" | ||
| check "snapshot includes learnings.md" "b" "$(cat "$snap/learnings.md")" | ||
| check "snapshot includes archive" "c" "$(cat "$snap/learnings-archive-2026-07-12.md")" | ||
| if grep -q "3 file" <<<"$out"; then | ||
| pass "--all reports file count" | ||
| else | ||
| fail "--all reports file count (got: $out)" | ||
| fi | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Usage / missing source → exit 2 (distinct from success). | ||
| # --------------------------------------------------------------------------- | ||
| check "no args exits 2" "2" "$(run)" | ||
| check "missing file exits 2" "2" "$(run "$tmp/does-not-exist.md")" | ||
| check "missing dir with --all exits 2" "2" "$(run --all "$tmp/no-such-dir")" | ||
|
|
||
| empty="$tmp/empty" | ||
| mkdir -p "$empty" | ||
| check "empty store with --all exits 2" "2" "$(run --all "$empty")" | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Custom --backup-dir is honoured. | ||
| # --------------------------------------------------------------------------- | ||
| store="$tmp/custom" | ||
| alt="$tmp/alt-backups" | ||
| mkdir -p "$store" | ||
| printf 'x\n' > "$store/portfolio-status.md" | ||
| export MEMORY_BACKUP_TS=20260724T003002Z | ||
| check "custom --backup-dir exits 0" "0" "$(run --backup-dir "$alt" "$store/portfolio-status.md")" | ||
| if [[ -f "$alt/portfolio-status.md.${MEMORY_BACKUP_TS}" ]]; then | ||
| pass "backup lands in --backup-dir" | ||
| else | ||
| fail "backup lands in --backup-dir" | ||
| fi | ||
|
|
||
| if [[ "$failures" -eq 0 ]]; then | ||
| printf '\nAll memory-backup self-tests passed.\n' | ||
| exit 0 | ||
| fi | ||
| printf '\n%d memory-backup self-test(s) FAILED.\n' "$failures" | ||
| exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When two instances select the same destination—identical basename, backup directory, and timestamp—both can pass the earlier existence check, and
mv -flets the later process silently replace the first backup. Running two such backups concurrently made both exit successfully and print restore commands, while only the second source's bytes remained, so the first command would restore incorrect data. Publish with an atomic no-replace operation or lock and report a collision instead.AGENTS.md reference: AGENTS.md:L2706-L2709
Useful? React with 👍 / 👎.