mirror of
https://github.com/addyosmani/agent-skills.git
synced 2026-10-26 15:05:04 +00:00
fix(hooks): simplify-ignore keeps a change reverted with git and re-hides the file
This commit is contained in:
1 parent
1401c8b803
commit
e9e172d909
3 files changed
+94
-7
No files matched your search
@@ -54,7 +54,7 @@ One script, three hook events:
|
||||
|---|---|
|
||||
| `PreToolUse Read` | Backs up file, replaces blocks with `BLOCK_<hash>` placeholders in-place |
|
||||
| `PostToolUse Edit\|Write` | Expands placeholders back to real code, saves model's changes, re-filters |
|
||||
| `Stop` | Expands the placeholders left in each file on disk, so changes made outside `Edit`/`Write` survive; falls back to the backup only if no placeholder is left |
|
||||
| `Stop` | Expands the placeholders left in each file on disk, so changes made outside `Edit`/`Write` survive; keeps a file that was put back whole (a `git checkout`/`stash` revert, an undo) instead of the stale backup; falls back to the backup only if no placeholder is left |
|
||||
|
||||
Each block is content-hashed (8 hex chars via `shasum`/`sha1sum`) so the round-trip is unambiguous even if the model duplicates or reorders placeholders. Cache is project-scoped to prevent cross-session interference.
|
||||
|
||||
|
||||
@@ -466,8 +466,51 @@ else
|
||||
assert_eq "Stop restores original + edit byte-identically" "identical" "differs"
|
||||
fi
|
||||
|
||||
# -- Test 17: a change reverted with git stays reverted and the block hidden --
|
||||
printf '\nTest 17: Reverting a change with git during the session\n'
|
||||
if command -v git >/dev/null 2>&1; then
|
||||
for variant in read-then-stop stop; do
|
||||
PROJ="$TMPDIR/rt17-$variant"; mkdir -p "$PROJ"
|
||||
RT="$PROJ/reverted.js"
|
||||
cat > "$RT" <<'EOF'
|
||||
const a = 1;
|
||||
// simplify-ignore-start
|
||||
const secret = 42;
|
||||
// simplify-ignore-end
|
||||
const b = 2;
|
||||
EOF
|
||||
cp "$RT" "$PROJ/reverted.orig"
|
||||
git -C "$PROJ" init -q
|
||||
git -C "$PROJ" config core.autocrlf false
|
||||
git -C "$PROJ" add reverted.js
|
||||
git -C "$PROJ" -c user.name=t -c user.email=t@t -c commit.gpgsign=false commit -qm base
|
||||
|
||||
rt_hook_event "$PROJ" "Read" "$RT"
|
||||
printf '%s\n' "const broken = true;" >> "$RT"
|
||||
rt_hook_event "$PROJ" "Edit" "$RT"
|
||||
# The skill's "if tests fail, revert that change", done with git
|
||||
git -C "$PROJ" checkout -q -- reverted.js
|
||||
if [ "$variant" = read-then-stop ]; then
|
||||
rt_hook_event "$PROJ" "Read" "$RT" 2>/dev/null
|
||||
assert_eq "$variant: Read after the revert hides the block" "0" "$(grep -c 'secret' "$RT")"
|
||||
assert_eq "$variant: Read after the revert shows a placeholder" "1" "$(grep -c 'BLOCK_' "$RT")"
|
||||
fi
|
||||
rt_hook_event "$PROJ" "" "" 2>/dev/null
|
||||
if cmp -s "$RT" "$PROJ/reverted.orig"; then
|
||||
assert_eq "$variant: Stop keeps the reverted file" "identical" "identical"
|
||||
else
|
||||
assert_eq "$variant: Stop keeps the reverted file" "identical" "differs"
|
||||
fi
|
||||
KEPT=$(ls "$PROJ/.claude/.simplify-ignore-cache"/*.superseded.* 2>/dev/null | head -1 || true)
|
||||
assert_eq "$variant: the reverted session state is kept in the cache" "1" \
|
||||
"$(grep -c 'const broken' "${KEPT:-/dev/null}")"
|
||||
done
|
||||
else
|
||||
printf '\nTests 14-16 skipped: jq not available (hook no-ops without it)\n'
|
||||
printf ' skipped: git not available\n'
|
||||
fi
|
||||
|
||||
else
|
||||
printf '\nTests 14-17 skipped: jq not available (hook no-ops without it)\n'
|
||||
fi
|
||||
|
||||
# ── Summary ──────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -232,6 +232,36 @@ has_placeholders() {
|
||||
return 1
|
||||
}
|
||||
|
||||
# -- file_is_whole: was the real file put back on disk while it was filtered? --
|
||||
# True when no placeholder of $2 is in file $1 and every cached block is there
|
||||
# verbatim (blocks are content-hashed, so filtering a probe copy compares them
|
||||
# exactly). A git checkout, restore, stash or reset, or an editor undo, does
|
||||
# this; the model cannot, because it never saw the block text.
|
||||
file_is_whole() {
|
||||
local f="$1" fid="$2" probe bf whole=0
|
||||
probe="${fid}.probe.$$"
|
||||
has_placeholders "$f" "$fid" && return 1
|
||||
grep -q 'simplify-ignore-start' -- "$f" 2>/dev/null || return 1
|
||||
if filter_file "$f" "$CACHE/${probe}.tmp" "$probe" 2>/dev/null; then
|
||||
whole=1
|
||||
for bf in "$CACHE/${fid}".block.*; do
|
||||
[ -f "$bf" ] && [ -f "$CACHE/${probe}.block.${bf##*.}" ] || { whole=0; break; }
|
||||
done
|
||||
fi
|
||||
rm -f "$CACHE/${probe}".*
|
||||
[ "$whole" -eq 1 ]
|
||||
}
|
||||
|
||||
# -- set_aside_backup: the file on disk supersedes the backup; keep a copy ----
|
||||
set_aside_backup() {
|
||||
local f="$1" fid="$2" kept
|
||||
cmp -s "$CACHE/${fid}.bak" "$f" && return 0
|
||||
kept="$CACHE/${fid}.superseded.$$"
|
||||
cp "$CACHE/${fid}.bak" "$kept"
|
||||
printf 'simplify-ignore: %s was put back whole while filtered (git checkout, stash, undo); keeping it. The previous session state is kept at %s\n' \
|
||||
"$f" "$kept" >&2
|
||||
}
|
||||
|
||||
# ── Stop: expand each cached file in place, backup only as fallback ──────────
|
||||
if [ -z "$TOOL_NAME" ]; then
|
||||
[ -d "$CACHE" ] || exit 0
|
||||
@@ -245,7 +275,12 @@ 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 file_is_whole "$orig" "$fid"; then
|
||||
# The real file is back (the change was reverted with git or undone).
|
||||
# It is newer than the backup, which would bring the reverted change
|
||||
# back, so keep the file and move the backup aside.
|
||||
set_aside_backup "$orig" "$fid"
|
||||
elif has_placeholders "$orig" "$fid"; then
|
||||
EXPANDED="$CACHE/${fid}.$$.expanded"
|
||||
expand_file "$orig" "$EXPANDED" "$fid"
|
||||
cat "$EXPANDED" > "$orig"
|
||||
@@ -291,12 +326,21 @@ if [ "$TOOL_NAME" = "Read" ]; then
|
||||
ID=$(file_id "$FILE_PATH")
|
||||
|
||||
# If backup exists, file is already filtered — skip
|
||||
[ -f "$CACHE/${ID}.bak" ] && exit 0
|
||||
|
||||
grep -q 'simplify-ignore-start' -- "$FILE_PATH" || exit 0
|
||||
# unless the real file was put back meanwhile (git checkout, stash, undo):
|
||||
# skipping then would show the model every protected block, so take that
|
||||
# file as the new original and filter it again.
|
||||
if [ -f "$CACHE/${ID}.bak" ]; then
|
||||
file_is_whole "$FILE_PATH" "$ID" || exit 0
|
||||
set_aside_backup "$FILE_PATH" "$ID"
|
||||
REFILTER=1
|
||||
else
|
||||
grep -q 'simplify-ignore-start' -- "$FILE_PATH" || exit 0
|
||||
REFILTER=0
|
||||
fi
|
||||
|
||||
# Atomic lock: mkdir fails if another session races us
|
||||
if ! mkdir "$CACHE/${ID}.lock" 2>/dev/null; then
|
||||
# (a refilter already holds the lock from the first Read)
|
||||
if [ "$REFILTER" -eq 0 ] && ! mkdir "$CACHE/${ID}.lock" 2>/dev/null; then
|
||||
# Lock exists — reclaim only if stale (>60s old, no backup = crash leftover)
|
||||
if [ ! -f "$CACHE/${ID}.bak" ] && \
|
||||
[ -n "$(find "$CACHE/${ID}.lock" -maxdepth 0 -mmin +1 2>/dev/null)" ]; then
|
||||
|
||||
Reference in new issue
Block a user