fix: distinguish phase file writes from reads in PostToolUse hook
- Parse tool_name via jq: Write tool checks file_path match, Bash tool checks for redirect operator (>) with phase file path - Reads (cat, head) no longer trigger false-positive markers - Split guard into separate statements for clarity - Move marker cleanup inside hook-install guard - Expand tests: 5 cases covering Bash write, Write tool, Bash read, unrelated Bash, and Write to different file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ac04dc29a6
commit
809dd93c3b
3 changed files with 67 additions and 18 deletions
|
|
@ -167,31 +167,61 @@ else
|
||||||
fail "needs_human mtime guard: expected 1 notify call, got $NOTIFY_COUNT"
|
fail "needs_human mtime guard: expected 1 notify call, got $NOTIFY_COUNT"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# ── Test 9: PostToolUse hook writes marker on phase file reference ────────
|
# ── Test 9: PostToolUse hook detects writes, ignores reads ────────────────
|
||||||
HOOK_SCRIPT="$(dirname "$0")/../lib/hooks/on-phase-change.sh"
|
HOOK_SCRIPT="$(dirname "$0")/../lib/hooks/on-phase-change.sh"
|
||||||
MARKER_FILE="/tmp/phase-changed-test-session.marker"
|
MARKER_FILE="/tmp/phase-changed-test-session.marker"
|
||||||
rm -f "$MARKER_FILE"
|
rm -f "$MARKER_FILE"
|
||||||
|
|
||||||
if [ -x "$HOOK_SCRIPT" ]; then
|
if [ -x "$HOOK_SCRIPT" ]; then
|
||||||
# Simulate hook input that references the phase file
|
# 9a: Bash redirect to phase file → marker written
|
||||||
echo "{\"tool_input\":{\"command\":\"echo PHASE:awaiting_ci > ${PHASE_FILE}\"}}" \
|
printf '{"tool_name":"Bash","tool_input":{"command":"echo PHASE:awaiting_ci > %s"}}' \
|
||||||
| bash "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
"$PHASE_FILE" | "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
||||||
if [ -f "$MARKER_FILE" ]; then
|
if [ -f "$MARKER_FILE" ]; then
|
||||||
ok "PostToolUse hook writes marker when phase file referenced"
|
ok "PostToolUse hook writes marker on Bash redirect to phase file"
|
||||||
else
|
else
|
||||||
fail "PostToolUse hook did not write marker"
|
fail "PostToolUse hook did not write marker on Bash redirect"
|
||||||
fi
|
fi
|
||||||
rm -f "$MARKER_FILE"
|
rm -f "$MARKER_FILE"
|
||||||
|
|
||||||
# Simulate hook input that does NOT reference the phase file
|
# 9b: Write tool targeting phase file → marker written
|
||||||
echo "{\"tool_input\":{\"command\":\"echo hello > /tmp/other-file\"}}" \
|
printf '{"tool_name":"Write","tool_input":{"file_path":"%s","content":"PHASE:done"}}' \
|
||||||
| bash "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
"$PHASE_FILE" | "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
||||||
|
if [ -f "$MARKER_FILE" ]; then
|
||||||
|
ok "PostToolUse hook writes marker on Write tool to phase file"
|
||||||
|
else
|
||||||
|
fail "PostToolUse hook did not write marker on Write tool"
|
||||||
|
fi
|
||||||
|
rm -f "$MARKER_FILE"
|
||||||
|
|
||||||
|
# 9c: Bash read of phase file (cat) → NO marker (not a write)
|
||||||
|
printf '{"tool_name":"Bash","tool_input":{"command":"cat %s"}}' \
|
||||||
|
"$PHASE_FILE" | "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
||||||
|
if [ ! -f "$MARKER_FILE" ]; then
|
||||||
|
ok "PostToolUse hook ignores Bash read of phase file (no false positive)"
|
||||||
|
else
|
||||||
|
fail "PostToolUse hook wrote marker for Bash read (false positive)"
|
||||||
|
fi
|
||||||
|
rm -f "$MARKER_FILE"
|
||||||
|
|
||||||
|
# 9d: Unrelated Bash command → NO marker
|
||||||
|
printf '{"tool_name":"Bash","tool_input":{"command":"echo hello > /tmp/other-file"}}' \
|
||||||
|
| "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
||||||
if [ ! -f "$MARKER_FILE" ]; then
|
if [ ! -f "$MARKER_FILE" ]; then
|
||||||
ok "PostToolUse hook skips marker for unrelated operations"
|
ok "PostToolUse hook skips marker for unrelated operations"
|
||||||
else
|
else
|
||||||
fail "PostToolUse hook wrote marker for unrelated operation (false positive)"
|
fail "PostToolUse hook wrote marker for unrelated operation (false positive)"
|
||||||
fi
|
fi
|
||||||
rm -f "$MARKER_FILE"
|
rm -f "$MARKER_FILE"
|
||||||
|
|
||||||
|
# 9e: Write tool targeting different file → NO marker
|
||||||
|
printf '{"tool_name":"Write","tool_input":{"file_path":"/tmp/other-file","content":"hello"}}' \
|
||||||
|
| "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE"
|
||||||
|
if [ ! -f "$MARKER_FILE" ]; then
|
||||||
|
ok "PostToolUse hook skips marker for Write to different file"
|
||||||
|
else
|
||||||
|
fail "PostToolUse hook wrote marker for Write to different file (false positive)"
|
||||||
|
fi
|
||||||
|
rm -f "$MARKER_FILE"
|
||||||
else
|
else
|
||||||
fail "PostToolUse hook script not found or not executable: $HOOK_SCRIPT"
|
fail "PostToolUse hook script not found or not executable: $HOOK_SCRIPT"
|
||||||
fi
|
fi
|
||||||
|
|
|
||||||
|
|
@ -110,9 +110,9 @@ create_agent_session() {
|
||||||
}
|
}
|
||||||
}' > "$settings"
|
}' > "$settings"
|
||||||
fi
|
fi
|
||||||
fi
|
|
||||||
rm -f "$phase_marker"
|
rm -f "$phase_marker"
|
||||||
fi
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
rm -f "$idle_marker"
|
rm -f "$idle_marker"
|
||||||
tmux new-session -d -s "$session" -c "$workdir" \
|
tmux new-session -d -s "$session" -c "$workdir" \
|
||||||
|
|
|
||||||
|
|
@ -2,9 +2,9 @@
|
||||||
# on-phase-change.sh — PostToolUse hook for phase file write detection.
|
# on-phase-change.sh — PostToolUse hook for phase file write detection.
|
||||||
#
|
#
|
||||||
# Called by Claude Code after every Bash|Write tool execution.
|
# Called by Claude Code after every Bash|Write tool execution.
|
||||||
# Checks if the tool input references the phase file path and, if so,
|
# Detects writes (not reads) to the phase file and writes a timestamp
|
||||||
# writes a "phase-changed" timestamp marker so monitor_phase_loop can
|
# marker so monitor_phase_loop can react immediately instead of waiting
|
||||||
# react immediately instead of waiting for the next mtime-based poll.
|
# for the next mtime-based poll.
|
||||||
#
|
#
|
||||||
# Usage (in .claude/settings.json):
|
# Usage (in .claude/settings.json):
|
||||||
# {"type": "command", "command": "this-script /path/to/phase-file /path/to/marker"}
|
# {"type": "command", "command": "this-script /path/to/phase-file /path/to/marker"}
|
||||||
|
|
@ -14,11 +14,30 @@
|
||||||
phase_file="${1:-}"
|
phase_file="${1:-}"
|
||||||
marker_file="${2:-}"
|
marker_file="${2:-}"
|
||||||
|
|
||||||
|
[ -z "$phase_file" ] && exit 0
|
||||||
|
[ -z "$marker_file" ] && exit 0
|
||||||
|
|
||||||
input=$(cat) # consume hook JSON from stdin
|
input=$(cat) # consume hook JSON from stdin
|
||||||
|
|
||||||
[ -z "$phase_file" ] || [ -z "$marker_file" ] && exit 0
|
# Fast path: skip if phase file not referenced at all
|
||||||
|
printf '%s' "$input" | grep -qF "$phase_file" || exit 0
|
||||||
|
|
||||||
# Check if the tool input references the phase file path
|
# Parse tool type and detect writes only (ignore reads like cat/head)
|
||||||
if printf '%s' "$input" | grep -qF "$phase_file"; then
|
tool_name=$(printf '%s' "$input" | jq -r '.tool_name // empty' 2>/dev/null)
|
||||||
|
|
||||||
|
case "$tool_name" in
|
||||||
|
Write)
|
||||||
|
# Write tool: check if file_path targets the phase file
|
||||||
|
file_path=$(printf '%s' "$input" | jq -r '.tool_input.file_path // empty' 2>/dev/null)
|
||||||
|
[ "$file_path" = "$phase_file" ] && date +%s > "$marker_file"
|
||||||
|
;;
|
||||||
|
Bash)
|
||||||
|
# Bash tool: check if the decoded command contains a redirect (>)
|
||||||
|
# targeting the phase file — distinguishes writes from reads
|
||||||
|
command_str=$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)
|
||||||
|
if printf '%s' "$command_str" | grep -qF "$phase_file" \
|
||||||
|
&& printf '%s' "$command_str" | grep -q '>'; then
|
||||||
date +%s > "$marker_file"
|
date +%s > "$marker_file"
|
||||||
fi
|
fi
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue