From a6563bc7d401246ce877717fca30f86fbc9b5283 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 24 Jul 2026 00:46:55 +0000 Subject: [PATCH] fix(memory): backup before destructive consolidate Add memory-backup.sh so size-threshold consolidations take a timestamped copy first (monorepo#2304), and point hygiene + contract guidance at that safe path. Co-authored-by: ned --- .claude/scripts/memory-backup.sh | 142 ++++++++++++++++++ .claude/scripts/memory-backup.test.sh | 122 +++++++++++++++ .claude/scripts/memory-hygiene-go/main.go | 4 + .../scripts/memory-hygiene-go/main_test.go | 2 +- .claude/skills/portfolio-maintenance/SKILL.md | 4 + .github/workflows/ci.yaml | 24 +++ AGENTS.md | 7 +- 7 files changed, 303 insertions(+), 2 deletions(-) create mode 100755 .claude/scripts/memory-backup.sh create mode 100755 .claude/scripts/memory-backup.test.sh diff --git a/.claude/scripts/memory-backup.sh b/.claude/scripts/memory-backup.sh new file mode 100755 index 00000000..105ab69c --- /dev/null +++ b/.claude/scripts/memory-backup.sh @@ -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 ] +# memory-backup.sh --all [--backup-dir ] +# +# Default backup root is /.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/. +# Whole-store layout: .memory-backups/store./ +# +# 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" +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/ %q/\n' "$store_dest" "$memory_dir" +exit 0 diff --git a/.claude/scripts/memory-backup.test.sh b/.claude/scripts/memory-backup.test.sh new file mode 100755 index 00000000..7578b19c --- /dev/null +++ b/.claude/scripts/memory-backup.test.sh @@ -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 diff --git a/.claude/scripts/memory-hygiene-go/main.go b/.claude/scripts/memory-hygiene-go/main.go index 5f2c1b43..4088f325 100644 --- a/.claude/scripts/memory-hygiene-go/main.go +++ b/.claude/scripts/memory-hygiene-go/main.go @@ -194,6 +194,10 @@ func run(args []string, stdout, stderr io.Writer) int { checked, ) output.line(" These will TRUNCATE at run start and silently hide carry-forwards.") + output.line(" BEFORE any destructive rewrite: .claude/scripts/memory-backup.sh ") + output.line( + " (or --all for a whole-store snapshot). Restore: cp '' ''.", + ) output.line(" Memory is a multi-writer surface: re-read immediately before writing and") output.line(" prefer a non-clobbering append over a whole-file rewrite.") } diff --git a/.claude/scripts/memory-hygiene-go/main_test.go b/.claude/scripts/memory-hygiene-go/main_test.go index c9c6783c..0bbe2886 100644 --- a/.claude/scripts/memory-hygiene-go/main_test.go +++ b/.claude/scripts/memory-hygiene-go/main_test.go @@ -179,7 +179,7 @@ func TestLegacyLayout(t *testing.T) { if code != 1 { t.Fatalf("exit code = %d, want 1", code) } - for _, expected := range []string{"OVER", "portfolio-status.md", "append"} { + for _, expected := range []string{"OVER", "portfolio-status.md", "append", "memory-backup.sh"} { if !strings.Contains(stdout, expected) { t.Fatalf("stdout %q does not contain %q", stdout, expected) } diff --git a/.claude/skills/portfolio-maintenance/SKILL.md b/.claude/skills/portfolio-maintenance/SKILL.md index ffe06e49..4cde6a01 100644 --- a/.claude/skills/portfolio-maintenance/SKILL.md +++ b/.claude/skills/portfolio-maintenance/SKILL.md @@ -98,6 +98,10 @@ card. through the runtime's supported path when needed and **restart the run** because this session did not start with the projection the guard checked; other exit-2 causes may rerun the guard in this session after resolution. + **Before any destructive rewrite of an author-managed (legacy) file**, take a timestamped copy: + `.claude/scripts/memory-backup.sh ` (or `--all ` for a whole-store snapshot). + Restore with `cp '' ''`. The store is un-versioned; a trim without a backup is + unrecoverable (monorepo#2304). Prefer append; rewrite only after that backup. **Memory is a MULTI-WRITER surface** — several instances append per hour. Re-read immediately before writing, prefer a **non-clobbering append** (`>>`) over a whole-file rewrite, and if a rewrite is rejected because the file moved under you, **stand down rather than clobber** a sibling's diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 40294cf5..91df2c73 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -33,6 +33,7 @@ jobs: board-add: ${{ steps.filter.outputs.board-add }} flow-scorecard: ${{ steps.filter.outputs.flow-scorecard }} memory-hygiene: ${{ steps.filter.outputs.memory-hygiene }} + memory-backup: ${{ steps.filter.outputs.memory-backup }} steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -100,6 +101,12 @@ jobs: # Self-gate on the workflow too, so a change that breaks this # job's own wiring still runs the test it gates. - '.github/workflows/ci.yaml' + memory-backup: + - '.claude/scripts/memory-backup.sh' + - '.claude/scripts/memory-backup.test.sh' + # Self-gate on the workflow too, so a change that breaks this + # job's own wiring still runs the test it gates. + - '.github/workflows/ci.yaml' maintainer-preflight: - '.claude/skills/portfolio-maintenance/SKILL.md' - '.claude/scripts/maintainer-preflight.test.sh' @@ -415,6 +422,21 @@ jobs: - name: Verify the memory hygiene guard run: bash .claude/scripts/memory-hygiene.test.sh + test-memory-backup: + name: Test memory backup helper + needs: changes + if: needs.changes.outputs.memory-backup == 'true' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - name: Verify the memory backup helper + run: bash .claude/scripts/memory-backup.test.sh + test-product-value-contract: name: Test product value contract needs: changes @@ -539,6 +561,7 @@ jobs: - test-submodule-init - test-maintainer-preflight - test-memory-hygiene + - test-memory-backup - test-self-review-contract - test-review-provider-loop-contract - test-agent-role-delivery-contract @@ -563,6 +586,7 @@ jobs: ${{ needs.test-submodule-init.result }} ${{ needs.test-maintainer-preflight.result }} ${{ needs.test-memory-hygiene.result }} + ${{ needs.test-memory-backup.result }} ${{ needs.test-self-review-contract.result }} ${{ needs.test-review-provider-loop-contract.result }} ${{ needs.test-agent-role-delivery-contract.result }} diff --git a/AGENTS.md b/AGENTS.md index 2787fa47..2b14a97e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2690,7 +2690,12 @@ step: diagnostic-only (`--all` shows the exemption). Legacy/Claude stores retain the original root-file checks. An exit 1 makes repairing the over-threshold boot-loaded file that tick's mandated hygiene item: consolidate an author-managed file safely, or refresh an oversized Codex projection through - the runtime. An exit 2 indicates a usage, malformed-layout, missing, or unreadable-store error; + the runtime. **Before any destructive consolidate/rewrite of an author-managed file**, run + [`.claude/scripts/memory-backup.sh`](.claude/scripts/memory-backup.sh) + `` (or `--all ` for a whole-store snapshot under `.memory-backups/`); restore with + `cp '' ''` — the store is un-versioned and outside git, so a trim without a backup is a + one-way delete (monorepo#2304). Prefer append; rewrite only when consolidating **after** that backup. + An exit 2 indicates a usage, malformed-layout, missing, or unreadable-store error; resolve it before proceeding. If a Codex exit 2 names a missing, unreadable, malformed, or post-injection-changed `memory_summary.md`, repair it through the runtime's supported path when needed and **restart the run**: this session did not start with the projection the guard checked.