Merge #649: simplify-ignore never loses a block whose placeholder was removed

hooks/SIMPLIFY-IGNORE.md

@@ -83,7 +83,7 @@ Backups are stored in `.claude/.simplify-ignore-cache/` within your project dire
 - **Single-line blocks hide the entire line.** If `simplify-ignore-start` and `simplify-ignore-end` appear on the same line as other code, the whole line is hidden from the model, not just the annotated portion. Use dedicated lines for annotations.
 - **Comment suffix detection covers `*/` and `-->` only.** Template engines with non-standard comment closers (ERB `%>`, Blade `--}}`) may produce unbalanced placeholders. Use `#` or `//` style comments instead.
 - **Fallback expansion is progressive, not exact.** If the model alters a placeholder's formatting (e.g. changes the reason text), the hook tries progressively simpler matches: full placeholder → prefix+hash+suffix → hash-only. The hash-only fallback may leave cosmetic debris (e.g. stray `:` or reason text). A warning is printed to stderr when this happens.
-- **A wholesale rewrite falls back to the backup.** If something replaces a filtered file's entire contents and removes every `BLOCK_<hash>` placeholder, the hook can no longer tell where the protected block belongs. At `Stop` it restores the backup and prints a warning; the rewrite is not lost but is kept in the cache as `.claude/.simplify-ignore-cache/<id>.recovered`, whose path the warning prints, and you merge it back by hand. Changes that leave the placeholders in place — a `Bash` edit, a formatter, an external editor — are kept in the file itself.
+- **A removed placeholder falls back to the backup.** If an edit deletes a `BLOCK_<hash>` line, or something replaces the whole file, the hook can no longer tell where that protected block belongs. An `Edit` or `Write` that does this is reported back to the model (exit 2) with the placeholder to restore, and the backup is not updated. If the placeholder is still missing at `Stop`, the hook restores the backup and prints a warning; the changed file is not lost but is kept in the cache as `.claude/.simplify-ignore-cache/<id>.recovered`, whose path the warning prints, and you merge it back by hand. Changes that leave the placeholders in place — a `Bash` edit, a formatter, an external editor — are kept in the file itself.
 - **File renaming leaves placeholders.** If the model renames or moves a file via a shell command, the new file will retain `BLOCK_<hash>` placeholders. The original code is saved as `<old-filename>.recovered` when the session stops. You must manually restore the recovered code into the new file.
 
 ## Requirements

hooks/simplify-ignore-test.sh

@@ -466,8 +466,58 @@ else
   assert_eq "Stop restores original + edit byte-identically" "identical" "differs"
 fi
 
+# ── Test 17: a removed placeholder never takes its protected block with it ─
+printf '\nTest 17: Removing a placeholder line does not lose the protected block\n'
+PROJ="$TMPDIR/rt17"; mkdir -p "$PROJ"
+RT="$PROJ/dropped.js"
+cat > "$RT" <<'EOF'
+const a = 1;
+// simplify-ignore-start: first
+const keepOne = 42;
+// simplify-ignore-end
+const b = 2;
+// simplify-ignore-start: second
+const keepTwo = 43;
+// simplify-ignore-end
+EOF
+cp "$RT" "$PROJ/dropped.orig"
+
+rt_hook_event "$PROJ" "Read" "$RT"
+# The model deletes the first placeholder line (it looks like a stray comment)
+# and edits another line in the same Edit.
+awk 'f||!/BLOCK_/{print;next}{f=1}' "$RT" | sed 's/const a = 1;/const a = 9;/' > "$PROJ/t"
+cat "$PROJ/t" > "$RT"
+edit_rc=0
+edit_out=$(rt_hook_event "$PROJ" "Edit" "$RT" 2>&1) || edit_rc=$?
+BAK=$(ls "$PROJ/.claude/.simplify-ignore-cache"/*.bak 2>/dev/null | head -1) || true
+assert_eq "Edit that drops a placeholder exits 2 (stderr goes to the model)" "2" "$edit_rc"
+assert_eq "message names the dropped placeholder" "1" "$(printf '%s' "$edit_out" | grep -c 'BLOCK_.*: first' || true)"
+assert_eq "backup still holds the protected block" "1" "$(grep -c 'const keepOne = 42' "${BAK:-/dev/null}" || true)"
+
+stop_out=$(rt_hook_event "$PROJ" "" "" 2>&1) || true
+RECOVERED=$(ls "$PROJ/.claude/.simplify-ignore-cache"/*.recovered 2>/dev/null | head -1) || true
+assert_eq "Stop puts the protected block back" "1" "$(grep -c 'const keepOne = 42' "$RT" || true)"
+if cmp -s "$RT" "$PROJ/dropped.orig"; then
+  assert_eq "Stop restores the last good backup" "identical" "identical"
 else
-  printf '\nTests 14-16 skipped: jq not available (hook no-ops without it)\n'
+  assert_eq "Stop restores the last good backup" "identical" "differs"
+fi
+assert_eq "the edit is kept as .recovered" "1" "$(grep -c 'const a = 9;' "${RECOVERED:-/dev/null}" || true)"
+assert_eq ".recovered has the surviving block expanded" "1" "$(grep -c 'const keepTwo = 43' "${RECOVERED:-/dev/null}" || true)"
+assert_eq "Stop warns about the missing placeholder" "1" "$(printf '%s' "$stop_out" | grep -c 'placeholder is missing' || true)"
+
+# Same loss through a route that fires no Edit event (Bash, a formatter).
+PROJ="$TMPDIR/rt17b"; mkdir -p "$PROJ"
+RT="$PROJ/dropped.js"
+cp "$TMPDIR/rt17/dropped.orig" "$RT"
+rt_hook_event "$PROJ" "Read" "$RT"
+awk 'f||!/BLOCK_/{print;next}{f=1}' "$RT" > "$PROJ/t"
+cat "$PROJ/t" > "$RT"
+rt_hook_event "$PROJ" "" "" 2>/dev/null || true
+assert_eq "no-event removal: Stop puts the protected block back" "1" "$(grep -c 'const keepOne = 42' "$RT" || true)"
+
+else
+  printf '\nTests 14-17 skipped: jq not available (hook no-ops without it)\n'
 fi
 
 # ── Summary ──────────────────────────────────────────────────────────────

hooks/simplify-ignore.sh

@@ -222,6 +222,27 @@ expand_file() {
 }
 
 # ── has_placeholders: is a cached BLOCK_<hash> for $2 still present in file $1? ─
+# -- missing_placeholders: which cached BLOCK_<hash> of $2 are gone from $1? --
+# Prints the missing placeholders, one per line, and returns 0 if any is gone.
+# A placeholder stands for code the model never saw, so it cannot have removed
+# it on purpose; whichever route removed it, the block must not be lost with it.
+missing_placeholders() {
+  local f="$1" fid="$2" bf h bp bs br out=""
+  for bf in "$CACHE/${fid}".block.*; do
+    [ -f "$bf" ] || continue
+    h="${bf##*.}"
+    grep -qF "BLOCK_${h}" "$f" 2>/dev/null && continue
+    bp=""; bs=""; br=""
+    [ -f "$CACHE/${fid}.prefix.${h}" ] && bp=$(cat "$CACHE/${fid}.prefix.${h}")
+    [ -f "$CACHE/${fid}.suffix.${h}" ] && bs=$(cat "$CACHE/${fid}.suffix.${h}")
+    [ -f "$CACHE/${fid}.reason.${h}" ] && br=": $(cat "$CACHE/${fid}.reason.${h}")"
+    out="${out}  ${bp}BLOCK_${h}${br}${bs}
+"
+  done
+  [ -n "$out" ] || return 1
+  printf '%s' "$out"
+}
+
 has_placeholders() {
   local f="$1" fid="$2" bf h
   for bf in "$CACHE/${fid}".block.*; do
@@ -245,20 +266,25 @@ if [ -z "$TOOL_NAME" ]; then
       # A change can reach the file through a route that fires no Edit or Write
       # event (a Bash command, a formatter, an external editor), and the backup
       # is stale for all of them. Expand what is on disk rather than overwrite it.
-      if has_placeholders "$orig" "$fid"; then
+      if has_placeholders "$orig" "$fid" && ! missing_placeholders "$orig" "$fid" >/dev/null; then
         EXPANDED="$CACHE/${fid}.$$.expanded"
         expand_file "$orig" "$EXPANDED" "$fid"
         cat "$EXPANDED" > "$orig"
         rm -f "$EXPANDED"
       else
-        # Nothing left to expand — the file was rewritten wholesale, so the
-        # backup is the only remaining copy of the protected blocks. Keep the
-        # rewrite in the cache before the backup overwrites it, so the work is
-        # recoverable rather than lost.
+        # A placeholder is gone — the file was rewritten wholesale, or one
+        # block's line was deleted — so the backup is the only remaining copy
+        # of that protected block. Keep the rewrite in the cache (with any
+        # surviving placeholders expanded) before the backup overwrites it, so
+        # the work is recoverable rather than lost.
         rewritten="$CACHE/${fid}.recovered"
-        cat "$orig" > "$rewritten"
+        if has_placeholders "$orig" "$fid"; then
+          expand_file "$orig" "$rewritten" "$fid" 2>/dev/null
+        else
+          cat "$orig" > "$rewritten"
+        fi
         cat "$bak" > "$orig"
-        printf 'Warning: no BLOCK_ placeholder left in %s, restored from backup. Rewrite kept at %s\n' \
+        printf 'Warning: a BLOCK_ placeholder is missing from %s, restored from backup. Rewrite kept at %s\n' \
           "$orig" "$rewritten" >&2
       fi
       rm -f "$bak" "$pathfile" "$CACHE/${fid}".block.* "$CACHE/${fid}".reason.* "$CACHE/${fid}".prefix.* "$CACHE/${fid}".suffix.*
@@ -330,6 +356,19 @@ if [ "$TOOL_NAME" = "Edit" ] || [ "$TOOL_NAME" = "Write" ]; then
   [ -f "$CACHE/${ID}.bak" ] || exit 0
   ls "$CACHE/${ID}".block.* >/dev/null 2>&1 || exit 0
 
+  # The edit removed a placeholder. Expanding now would save a backup without
+  # that block, and Stop would then restore the loss. Keep the last good
+  # backup, leave the file as edited, and tell the model (exit 2 sends stderr
+  # back to it) so it can put the placeholder back; if it does not, Stop
+  # restores the backup and keeps this version as <id>.recovered.
+  if dropped=$(missing_placeholders "$FILE_PATH" "$ID"); then
+    printf '%s\n%s\n%s\n' \
+      "simplify-ignore: this $TOOL_NAME removed placeholder line(s) from $FILE_PATH. Each stands for protected code you cannot see:" \
+      "$dropped" \
+      "Put each one back exactly as shown. Until then the backup is not updated, and at session end the file is restored from it (this version is kept in the cache as .recovered)." >&2
+    exit 2
+  fi
+
   # Expand placeholders, preserving any inline code the model added around them
   EXPANDED="$CACHE/${ID}.$$.expanded"
   expand_file "$FILE_PATH" "$EXPANDED" "$ID"