diff --git a/.claude/scripts/memory-hygiene-go/main.go b/.claude/scripts/memory-hygiene-go/main.go index 5f2c1b43..299be627 100644 --- a/.claude/scripts/memory-hygiene-go/main.go +++ b/.claude/scripts/memory-hygiene-go/main.go @@ -196,6 +196,7 @@ func run(args []string, stdout, stderr io.Writer) int { output.line(" These will TRUNCATE at run start and silently hide carry-forwards.") 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.") + output.line(" If a rewrite is required, use memory-rewrite.sh (never sed+mv into the live file).") } return output.exitCode(1) } diff --git a/.claude/scripts/memory-rewrite.sh b/.claude/scripts/memory-rewrite.sh new file mode 100755 index 00000000..9e9c44cd --- /dev/null +++ b/.claude/scripts/memory-rewrite.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# +# memory-rewrite.sh — fail-closed whole-file rewrite for the shared durable +# memory store (monorepo#2293). +# +# The footgun this closes: agents "surgically" trim a section with +# s=$(grep -n '^## …' "$f" | cut -d: -f1) +# { sed -n "1,$((s-1))p" "$f"; echo replacement; } > /tmp/new && mv /tmp/new "$f" +# When grep matches nothing (a sibling restructured the file), s is empty, +# sed gets `1,-1p`, errors and emits nothing, the `{…}` block still produces +# a near-empty stream, and `mv` permanently clobbers an unversioned file. +# Measured 2026-07-20: two independent ticks destroyed learnings.md and +# portfolio-status.md the same day; neither was recoverable. +# +# This helper backs up first, refuses empty/non-positive keep-through bounds, +# refuses empty or drastically-shrunk rebuilds (unless --allow-shrink), refuses +# a rebuild that drops a markdown heading the original had, and only then +# swaps. Prefer a non-clobbering append when you can; use this only when a +# whole-file rewrite is genuinely required. +# +# Usage: +# memory-rewrite.sh --file --from [options] +# memory-rewrite.sh --file --stdin [options] +# memory-rewrite.sh --file --keep-through --suffix [options] +# +# Options: +# --allow-shrink permit a rebuild smaller than --max-shrink-pct of the original +# --max-shrink-pct N refuse when new size < (100-N)% of old (default: 50) +# --backup-dir directory for the pre-swap backup (default: same dir as --file) +# +# Exit codes: +# 0 rewrite applied; stdout carries `backup=` +# 1 content refused (empty, shrink, lost heading) — target untouched +# 2 usage / bound error — target untouched +set -Eeuo pipefail + +DEFAULT_MAX_SHRINK_PCT=50 + +file="" +from="" +stdin=0 +keep_through="" +suffix="" +allow_shrink=0 +max_shrink_pct="$DEFAULT_MAX_SHRINK_PCT" +backup_dir="" + +usage() { + sed -n '/^# Usage:/,/^# 2 /p' "$0" | sed 's/^# \{0,1\}//' +} + +fail_usage() { + echo "memory-rewrite: $*" >&2 + usage >&2 + exit 2 +} + +fail_content() { + echo "memory-rewrite: $*" >&2 + exit 1 +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --file) file="${2-}"; shift 2 || fail_usage "--file needs a path" ;; + --from) from="${2-}"; shift 2 || fail_usage "--from needs a path" ;; + --stdin) stdin=1; shift ;; + --keep-through) keep_through="${2-}"; shift 2 || fail_usage "--keep-through needs a line number" ;; + --suffix) suffix="${2-}"; shift 2 || fail_usage "--suffix needs a path" ;; + --allow-shrink) allow_shrink=1; shift ;; + --max-shrink-pct) max_shrink_pct="${2-}"; shift 2 || fail_usage "--max-shrink-pct needs an integer" ;; + --backup-dir) backup_dir="${2-}"; shift 2 || fail_usage "--backup-dir needs a path" ;; + -h|--help) usage; exit 0 ;; + *) fail_usage "unknown argument '$1'" ;; + esac +done + +[[ -n "$file" ]] || fail_usage "--file is required" +[[ -f "$file" ]] || fail_usage "target file not found: $file" + +# Exactly one content source. +source_count=0 +[[ -n "$from" ]] && source_count=$((source_count + 1)) +[[ "$stdin" -eq 1 ]] && source_count=$((source_count + 1)) +[[ -n "$keep_through" || -n "$suffix" ]] && source_count=$((source_count + 1)) +[[ "$source_count" -eq 1 ]] || fail_usage "provide exactly one of --from, --stdin, or --keep-through/--suffix" + +if ! [[ "$max_shrink_pct" =~ ^[0-9]+$ ]]; then + fail_usage "--max-shrink-pct must be a non-negative integer (got '$max_shrink_pct')" +fi +printf -v max_shrink_pct '%d' "$((10#$max_shrink_pct))" +if [[ "$max_shrink_pct" -lt 0 || "$max_shrink_pct" -gt 99 ]]; then + fail_usage "--max-shrink-pct must be 0..99 (got '$max_shrink_pct')" +fi + +workdir="$(mktemp -d "${TMPDIR:-/tmp}/memory-rewrite.XXXXXX")" +cleanup() { rm -rf "$workdir"; } +trap cleanup EXIT + +new_path="$workdir/new" + +if [[ -n "$from" ]]; then + [[ -f "$from" ]] || fail_usage "--from file not found: $from" + cp "$from" "$new_path" +elif [[ "$stdin" -eq 1 ]]; then + cat > "$new_path" +else + # Assemble: lines 1..N of the target + suffix. Bound must be a positive integer — + # this is the empty-grep failure mode made explicit. + if [[ -z "$keep_through" ]]; then + fail_usage "--keep-through is empty or missing (refusing the sed 1,-1p footgun)" + fi + # Reject a leading minus before base-10 normalisation — bash `$((10#-3))` is a + # syntax error and would exit 1 under `set -e` instead of the usage exit 2. + if [[ "$keep_through" == -* ]]; then + fail_usage "--keep-through must be >= 1 (got '$keep_through'); refusing empty/non-positive bound" + fi + if ! [[ "$keep_through" =~ ^[0-9]+$ ]]; then + fail_usage "--keep-through must be an integer (got '$keep_through')" + fi + # Normalise leading zeros via 10#; reject non-positive. + printf -v keep_through '%d' "$((10#$keep_through))" + if [[ "$keep_through" -lt 1 ]]; then + fail_usage "--keep-through must be >= 1 (got '$keep_through'); refusing empty/non-positive bound" + fi + [[ -n "$suffix" ]] || fail_usage "--keep-through requires --suffix" + [[ -f "$suffix" ]] || fail_usage "--suffix file not found: $suffix" + target_lines="$(wc -l < "$file" | tr -d '[:space:]')" + if [[ "$keep_through" -gt "$target_lines" ]]; then + fail_usage "--keep-through $keep_through exceeds file line count $target_lines" + fi + { + sed -n "1,${keep_through}p" "$file" + cat "$suffix" + } > "$new_path" +fi + +old_bytes="$(wc -c < "$file" | tr -d '[:space:]')" +new_bytes="$(wc -c < "$new_path" | tr -d '[:space:]')" + +if [[ "$new_bytes" -eq 0 ]]; then + fail_content "refused empty rebuild of $file (new content is 0 bytes); target untouched" +fi + +if [[ "$allow_shrink" -eq 0 ]]; then + # Keep at least (100 - max_shrink_pct)% of the original size. + min_bytes=$(( old_bytes * (100 - max_shrink_pct) / 100 )) + # Always require at least 1 byte when old was non-empty (covered above). + if [[ "$old_bytes" -gt 0 && "$new_bytes" -lt "$min_bytes" ]]; then + fail_content "refused drastic shrink of $file: new=${new_bytes}B old=${old_bytes}B (below $((100 - max_shrink_pct))% keep); re-run with --allow-shrink if intentional" + fi +fi + +# Structure guard: if the original had a markdown heading, the rebuild must too. +if grep -qE '^#{1,6}[[:space:]]' "$file"; then + if ! grep -qE '^#{1,6}[[:space:]]' "$new_path"; then + fail_content "refused rebuild of $file: original had a markdown heading and the new content does not; target untouched" + fi +fi + +# Frontmatter guard: if the original opened with YAML frontmatter, keep it. +if head -n 1 "$file" | grep -qx -- '---'; then + if ! head -n 1 "$new_path" | grep -qx -- '---'; then + fail_content "refused rebuild of $file: original had YAML frontmatter and the new content does not; target untouched" + fi +fi + +# Backup first, then atomic-ish replace via temp + mv in the same directory. +if [[ -z "$backup_dir" ]]; then + backup_dir="$(dirname "$file")" +fi +mkdir -p "$backup_dir" +stamp="$(date -u +%Y%m%dT%H%M%SZ)" +base="$(basename "$file")" +backup_path="$backup_dir/${base}.bak.${stamp}.$$" +cp "$file" "$backup_path" + +swap="$file.memory-rewrite.$$" +cp "$new_path" "$swap" +mv "$swap" "$file" + +printf 'memory-rewrite: ok file=%s backup=%s new_bytes=%s\n' "$file" "$backup_path" "$new_bytes" +exit 0 diff --git a/.claude/scripts/memory-rewrite.test.sh b/.claude/scripts/memory-rewrite.test.sh new file mode 100755 index 00000000..8467164c --- /dev/null +++ b/.claude/scripts/memory-rewrite.test.sh @@ -0,0 +1,179 @@ +#!/usr/bin/env bash +# +# Self-test for memory-rewrite.sh — RED-proves the two real clobber modes from +# monorepo#2293: an empty/non-positive keep-through bound (the sed `1,-1p` +# footgun), and a rebuild that is empty or drastically smaller than the +# original. Also asserts backups are written and reported, and that an +# intentional shrink needs --allow-shrink. +# +# 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-rewrite.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 +} + +run() { local rc=0; "$tool" "$@" >/dev/null 2>&1 || rc=$?; echo "$rc"; } + +# --------------------------------------------------------------------------- +# Happy path: full rewrite from --from keeps content and reports a backup. +# --------------------------------------------------------------------------- +target="$tmp/happy.md" +printf '%s\n' '---' 'version: 1' '---' '# Notes' '' 'old body line' > "$target" +new="$tmp/happy-new.md" +printf '%s\n' '---' 'version: 1' '---' '# Notes' '' 'new body line' 'another' > "$new" +rc=0 +out="$("$tool" --file "$target" --from "$new" 2>&1)" || rc=$? +check "happy rewrite exit code" "0" "$rc" +if grep -q 'new body line' "$target" && grep -q '# Notes' "$target"; then + pass "happy rewrite installs new content" +else + fail "happy rewrite installs new content" +fi +if grep -q 'backup=' <<<"$out"; then + pass "happy rewrite reports backup= path" + bak="$(sed -n 's/.*backup=//p' <<<"$out" | awk '{print $1}' | head -1)" + if [[ -f "$bak" ]] && grep -q 'old body line' "$bak"; then + pass "backup file preserves prior content" + else + fail "backup file preserves prior content (bak='$bak')" + fi +else + fail "happy rewrite reports backup= path (got: $out)" +fi + +# --------------------------------------------------------------------------- +# RED case 1: empty / non-positive --keep-through bound must refuse BEFORE +# touching the target (the empty-grep → sed 1,-1p clobber). +# --------------------------------------------------------------------------- +target="$tmp/bound.md" +printf '%s\n' '# Heading' 'line2' 'line3' 'line4' 'line5' > "$target" +before="$(cat "$target")" +suffix="$tmp/suffix.md" +printf '%s\n' 'replacement section' > "$suffix" + +rc=0 +out="$("$tool" --file "$target" --keep-through '' --suffix "$suffix" 2>&1)" || rc=$? +check "empty keep-through exits non-zero" "2" "$rc" +check "empty keep-through leaves target untouched" "$before" "$(cat "$target")" +if grep -qi 'keep-through' <<<"$out"; then + pass "empty keep-through names the bound in the error" +else + fail "empty keep-through names the bound in the error (got: $out)" +fi + +rc=0 +out="$("$tool" --file "$target" --keep-through 0 --suffix "$suffix" 2>&1)" || rc=$? +check "zero keep-through exits non-zero" "2" "$rc" +check "zero keep-through leaves target untouched" "$before" "$(cat "$target")" + +rc=0 +out="$("$tool" --file "$target" --keep-through -3 --suffix "$suffix" 2>&1)" || rc=$? +check "negative keep-through exits non-zero" "2" "$rc" +check "negative keep-through leaves target untouched" "$before" "$(cat "$target")" + +# --------------------------------------------------------------------------- +# RED case 2: empty / near-empty rebuild must refuse (the sed-error → empty +# `{…}` block still producing output, then mv clobber). +# --------------------------------------------------------------------------- +target="$tmp/empty-rebuild.md" +{ + printf '%s\n' '---' 'version: 1' '---' '# Portfolio status' + local_i=0 + for (( local_i = 0; local_i < 40; local_i++ )); do + printf 'tick note line %s with enough bytes to matter for shrink detection\n' "$local_i" + done +} > "$target" +before="$(cat "$target")" +empty_new="$tmp/empty-new.md" +: > "$empty_new" + +rc=0 +out="$("$tool" --file "$target" --from "$empty_new" 2>&1)" || rc=$? +check "empty rebuild exits non-zero" "1" "$rc" +check "empty rebuild leaves target untouched" "$before" "$(cat "$target")" +if grep -qiE 'empty|refuse|rejected' <<<"$out"; then + pass "empty rebuild error mentions refusal" +else + fail "empty rebuild error mentions refusal (got: $out)" +fi + +tiny_new="$tmp/tiny-new.md" +# Keep frontmatter + heading so only the shrink guard fires (not structure). +printf '%s\n' '---' 'version: 1' '---' '# Portfolio status' 'x' > "$tiny_new" +rc=0 +out="$("$tool" --file "$target" --from "$tiny_new" 2>&1)" || rc=$? +check "drastic shrink exits non-zero by default" "1" "$rc" +check "drastic shrink leaves target untouched" "$before" "$(cat "$target")" +if grep -qiE 'shrink|smaller|allow-shrink' <<<"$out"; then + pass "drastic shrink error mentions shrink / allow-shrink" +else + fail "drastic shrink error mentions shrink / allow-shrink (got: $out)" +fi + +# Opt-in shrink is allowed when explicitly requested (and still backs up). +rc=0 +out="$("$tool" --file "$target" --from "$tiny_new" --allow-shrink 2>&1)" || rc=$? +check "allow-shrink permits intentional shrink" "0" "$rc" +if grep -q '^# Portfolio status$' "$target" && grep -q 'backup=' <<<"$out"; then + pass "allow-shrink installs tiny content with backup" +else + fail "allow-shrink installs tiny content with backup (got out: $out / file: $(cat "$target"))" +fi + +# --------------------------------------------------------------------------- +# keep-through assemble path succeeds with a positive bound. +# --------------------------------------------------------------------------- +target="$tmp/assemble.md" +printf '%s\n' '# Heading' 'keep-me-1' 'keep-me-2' 'drop-me' 'drop-me-2' > "$target" +suffix="$tmp/suffix2.md" +printf '%s\n' '## Replacement' 'fresh content' > "$suffix" +rc=0 +out="$("$tool" --file "$target" --keep-through 3 --suffix "$suffix" 2>&1)" || rc=$? +check "assemble with positive keep-through exits 0" "0" "$rc" +if grep -q 'keep-me-2' "$target" && grep -q 'fresh content' "$target" && ! grep -q 'drop-me' "$target"; then + pass "assemble keeps prefix and appends suffix" +else + fail "assemble keeps prefix and appends suffix (got: $(cat "$target"))" +fi + +# --------------------------------------------------------------------------- +# Structure guard: original had a markdown heading; new content without one +# is refused. +# --------------------------------------------------------------------------- +target="$tmp/heading.md" +printf '%s\n' '# Real heading' 'body body body body body body body body' > "$target" +before="$(cat "$target")" +nohead="$tmp/nohead.md" +printf '%s\n' 'just prose with no heading at all, padded enough not to trip shrink alone.......' > "$nohead" +rc=0 +out="$("$tool" --file "$target" --from "$nohead" 2>&1)" || rc=$? +check "lost heading exits non-zero" "1" "$rc" +check "lost heading leaves target untouched" "$before" "$(cat "$target")" + +# --------------------------------------------------------------------------- +# Usage errors. +# --------------------------------------------------------------------------- +check "missing --file exits 2" "2" "$(run --from "$new")" +check "missing content source exits 2" "2" "$(run --file "$tmp/happy.md")" + +# --------------------------------------------------------------------------- +if [[ "$failures" -eq 0 ]]; then + printf '\nAll memory-rewrite tests passed.\n' + exit 0 +fi +printf '\n%d memory-rewrite test(s) FAILED.\n' "$failures" +exit 1 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 14b1313e..c13e913d 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-rewrite: ${{ steps.filter.outputs.memory-rewrite }} steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -100,6 +101,10 @@ 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-rewrite: + - '.claude/scripts/memory-rewrite.sh' + - '.claude/scripts/memory-rewrite.test.sh' + - '.github/workflows/ci.yaml' maintainer-preflight: - '.claude/skills/portfolio-maintenance/SKILL.md' - '.claude/scripts/maintainer-preflight.test.sh' @@ -415,6 +420,21 @@ jobs: - name: Verify the memory hygiene guard run: bash .claude/scripts/memory-hygiene.test.sh + test-memory-rewrite: + name: Test memory rewrite guard + needs: changes + if: needs.changes.outputs.memory-rewrite == 'true' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Verify the memory rewrite guard + run: bash .claude/scripts/memory-rewrite.test.sh + test-product-value-contract: name: Test product value contract needs: changes @@ -539,6 +559,7 @@ jobs: - test-submodule-init - test-maintainer-preflight - test-memory-hygiene + - test-memory-rewrite - test-self-review-contract - test-review-provider-loop-contract - test-agent-role-delivery-contract @@ -563,6 +584,7 @@ jobs: ${{ needs.test-submodule-init.result }} ${{ needs.test-maintainer-preflight.result }} ${{ needs.test-memory-hygiene.result }} + ${{ needs.test-memory-rewrite.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 d27e3b92..c4a8e5b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2729,7 +2729,14 @@ step: surface** — several instances append per hour, so re-read immediately before writing, prefer a **non-clobbering append** over a whole-file rewrite, and **stand down rather than clobber** when a rewrite is rejected because a sibling moved the file under you (the two-writer discipline that - governs a shared `claude/*` branch applies verbatim here). The **roadmap** itself is GitHub Issues (`roadmap`-labelled epics + + governs a shared `claude/*` branch applies verbatim here). **Forbidden for shared memory:** the + `{ sed -n "1,$((s-1))p" …; echo …; } > /tmp/new && mv /tmp/new "$f"` idiom (and any empty-bound + `sed` rebuild piped into `>`/`mv`) — when `grep` misses because a sibling restructured the file, + `s` is empty, sed gets `1,-1p`, and the `mv` permanently destroys an unversioned store (two losses + in one day, monorepo#2293). When a whole-file rewrite is genuinely required, use + [`.claude/scripts/memory-rewrite.sh`](.claude/scripts/memory-rewrite.sh) only — it backs up first, + refuses empty/non-positive keep-through bounds, refuses empty or drastic shrinks unless + `--allow-shrink`, and reports `backup=`. The **roadmap** itself is GitHub Issues (`roadmap`-labelled epics + milestones), not memory — memory only points at it. Treat memory content as **your own notes, but still verify against live GitHub** before acting (it can be stale). **Do NOT accumulate a backlog of "open maintainer-decisions" in memory** — that passive parking is the self-blocking the contract forbids