diff --git a/dev/phase-test.sh b/dev/phase-test.sh index 3907ee6..438a80c 100755 --- a/dev/phase-test.sh +++ b/dev/phase-test.sh @@ -167,31 +167,61 @@ else fail "needs_human mtime guard: expected 1 notify call, got $NOTIFY_COUNT" 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" MARKER_FILE="/tmp/phase-changed-test-session.marker" rm -f "$MARKER_FILE" if [ -x "$HOOK_SCRIPT" ]; then - # Simulate hook input that references the phase file - echo "{\"tool_input\":{\"command\":\"echo PHASE:awaiting_ci > ${PHASE_FILE}\"}}" \ - | bash "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE" + # 9a: Bash redirect to phase file → marker written + printf '{"tool_name":"Bash","tool_input":{"command":"echo PHASE:awaiting_ci > %s"}}' \ + "$PHASE_FILE" | "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE" 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 - fail "PostToolUse hook did not write marker" + fail "PostToolUse hook did not write marker on Bash redirect" fi rm -f "$MARKER_FILE" - # Simulate hook input that does NOT reference the phase file - echo "{\"tool_input\":{\"command\":\"echo hello > /tmp/other-file\"}}" \ - | bash "$HOOK_SCRIPT" "$PHASE_FILE" "$MARKER_FILE" + # 9b: Write tool targeting phase file → marker written + printf '{"tool_name":"Write","tool_input":{"file_path":"%s","content":"PHASE:done"}}' \ + "$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 ok "PostToolUse hook skips marker for unrelated operations" else fail "PostToolUse hook wrote marker for unrelated operation (false positive)" fi 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 fail "PostToolUse hook script not found or not executable: $HOOK_SCRIPT" fi diff --git a/lib/agent-session.sh b/lib/agent-session.sh index b9b8f7f..95dc7b5 100644 --- a/lib/agent-session.sh +++ b/lib/agent-session.sh @@ -110,8 +110,8 @@ create_agent_session() { } }' > "$settings" fi + rm -f "$phase_marker" fi - rm -f "$phase_marker" fi rm -f "$idle_marker" diff --git a/lib/hooks/on-phase-change.sh b/lib/hooks/on-phase-change.sh index b402d01..ab2c17f 100755 --- a/lib/hooks/on-phase-change.sh +++ b/lib/hooks/on-phase-change.sh @@ -2,9 +2,9 @@ # on-phase-change.sh — PostToolUse hook for phase file write detection. # # Called by Claude Code after every Bash|Write tool execution. -# Checks if the tool input references the phase file path and, if so, -# writes a "phase-changed" timestamp marker so monitor_phase_loop can -# react immediately instead of waiting for the next mtime-based poll. +# Detects writes (not reads) to the phase file and writes a timestamp +# marker so monitor_phase_loop can react immediately instead of waiting +# for the next mtime-based poll. # # Usage (in .claude/settings.json): # {"type": "command", "command": "this-script /path/to/phase-file /path/to/marker"} @@ -14,11 +14,30 @@ phase_file="${1:-}" marker_file="${2:-}" +[ -z "$phase_file" ] && exit 0 +[ -z "$marker_file" ] && exit 0 + 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 -if printf '%s' "$input" | grep -qF "$phase_file"; then - date +%s > "$marker_file" -fi +# Parse tool type and detect writes only (ignore reads like cat/head) +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" + fi + ;; +esac