mirror of
https://github.com/addyosmani/agent-skills.git
synced 2026-10-26 15:05:04 +00:00
fix(hooks): keep the rewrite in the cache when simplify-ignore falls back
When a Stop leaves no BLOCK_ placeholder to expand, the hook has to restore the backup to recover the protected block — which overwrote whatever had replaced the file. Copy that content into the cache as <id>.recovered first and name both paths in the warning, so the rewrite is merged back by hand rather than lost. Adds a test that the fallback keeps the rewritten content in the cache, and a minimal lifecycle test (Read, a direct write with no event, Stop) asserting the write survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
b1ac22c7d2
commit
510259aca0
3 files changed
+84
-3
No files matched your search
@@ -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, so that rewrite is lost. Changes that leave the placeholders in place — a `Bash` edit, a formatter, an external editor — are kept.
|
||||
- **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.
|
||||
- **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
|
||||
|
||||
@@ -291,6 +291,82 @@ EOF
|
||||
assert_eq "no placeholder left after Stop" "0" "$(grep -c 'BLOCK_' "$TARGET")"
|
||||
fi
|
||||
|
||||
# ── Test 12: Stop fallback keeps the rewrite in the cache ───────────────
|
||||
printf '\nTest 12: Wholesale rewrite is kept in the cache before the backup restore\n'
|
||||
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
printf ' SKIP: full lifecycle needs jq (the hook exits at its jq guard)\n'
|
||||
else
|
||||
PROJ="$TMPDIR/proj-rewrite"
|
||||
rm -rf "$PROJ"
|
||||
mkdir -p "$PROJ/.claude"
|
||||
TARGET="$PROJ/app.js"
|
||||
cat > "$TARGET" <<'EOF'
|
||||
const editable = 1;
|
||||
/* simplify-ignore-start: perf-critical */
|
||||
const protectedValue = 42;
|
||||
/* simplify-ignore-end */
|
||||
EOF
|
||||
|
||||
hook_event() { printf '%s' "$1" | CLAUDE_PROJECT_DIR="$PROJ" bash hooks/simplify-ignore.sh; }
|
||||
read_event=$(printf '{"tool_name":"Read","tool_input":{"file_path":"%s"}}' "$TARGET")
|
||||
|
||||
hook_event "$read_event"
|
||||
assert_eq "protected block hidden after Read" "1" "$(grep -c 'BLOCK_' "$TARGET")"
|
||||
|
||||
# Wholesale rewrite — every placeholder is gone, so Stop has nothing to expand
|
||||
printf 'const rewritten = 7;\n' > "$TARGET"
|
||||
|
||||
stop_out=$(hook_event '{}' 2>&1) || true
|
||||
|
||||
PROJ_CACHE="$PROJ/.claude/.simplify-ignore-cache"
|
||||
RECOVERED="$PROJ_CACHE/$(file_id "$TARGET").recovered"
|
||||
|
||||
assert_eq "backup restored over the rewrite" "1" "$(grep -c 'const protectedValue = 42;' "$TARGET")"
|
||||
assert_eq "rewrite kept in cache" "1" "$([ -f "$RECOVERED" ] && echo 1 || echo 0)"
|
||||
assert_eq "cached rewrite has the rewritten content" "const rewritten = 7;" "$(cat "$RECOVERED")"
|
||||
assert_eq "warning names the restored file" "1" \
|
||||
"$(printf '%s' "$stop_out" | grep -c -F "$TARGET")"
|
||||
assert_eq "warning names the cached rewrite" "1" \
|
||||
"$(printf '%s' "$stop_out" | grep -c -F "$RECOVERED")"
|
||||
fi
|
||||
|
||||
# ── Test 13: Read → direct write with no event → Stop ───────────────────
|
||||
printf '\nTest 13: A write with no Edit or Write event survives Stop\n'
|
||||
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
printf ' SKIP: full lifecycle needs jq (the hook exits at its jq guard)\n'
|
||||
else
|
||||
PROJ="$TMPDIR/proj-noevent"
|
||||
rm -rf "$PROJ"
|
||||
mkdir -p "$PROJ/.claude"
|
||||
TARGET="$PROJ/app.js"
|
||||
cat > "$TARGET" <<'EOF'
|
||||
const editable = 1;
|
||||
/* simplify-ignore-start: perf-critical */
|
||||
const protectedValue = 42;
|
||||
/* simplify-ignore-end */
|
||||
EOF
|
||||
|
||||
hook_event() { printf '%s' "$1" | CLAUDE_PROJECT_DIR="$PROJ" bash hooks/simplify-ignore.sh; }
|
||||
read_event=$(printf '{"tool_name":"Read","tool_input":{"file_path":"%s"}}' "$TARGET")
|
||||
|
||||
# Read → placeholder on disk, backup taken
|
||||
hook_event "$read_event"
|
||||
|
||||
# Direct write, no Edit or Write event follows it
|
||||
sed 's/const editable = 1;/const editable = 9;/' "$TARGET" > "$TMPDIR/noevent-write.js"
|
||||
cat "$TMPDIR/noevent-write.js" > "$TARGET"
|
||||
|
||||
# Stop
|
||||
hook_event '{}'
|
||||
|
||||
assert_eq "write with no event survives Stop" "1" "$(grep -c 'const editable = 9;' "$TARGET")"
|
||||
assert_eq "stale backup value not restored" "0" "$(grep -c 'const editable = 1;' "$TARGET")"
|
||||
assert_eq "protected block back on disk" "1" "$(grep -c 'const protectedValue = 42;' "$TARGET")"
|
||||
assert_eq "no placeholder left after Stop" "0" "$(grep -c 'BLOCK_' "$TARGET")"
|
||||
fi
|
||||
|
||||
# ── Summary ──────────────────────────────────────────────────────────────
|
||||
printf '\n══════════════════════════════════════════\n'
|
||||
printf 'Results: %d passed, %d failed\n' "$PASS" "$FAIL"
|
||||
|
||||
@@ -244,9 +244,14 @@ if [ -z "$TOOL_NAME" ]; then
|
||||
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.
|
||||
printf 'Warning: no BLOCK_ placeholder left in %s, restored from backup\n' "$orig" >&2
|
||||
# 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.
|
||||
rewritten="$CACHE/${fid}.recovered"
|
||||
cat "$orig" > "$rewritten"
|
||||
cat "$bak" > "$orig"
|
||||
printf 'Warning: no BLOCK_ placeholder left in %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.*
|
||||
rmdir "$CACHE/${fid}.lock" 2>/dev/null
|
||||
|
||||
Reference in new issue
Block a user