From ac04dc29a6f5beb521be75ec282711238449db52 Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 19 Mar 2026 17:55:06 +0000 Subject: [PATCH] fix: feat: PostToolUse hook detects phase file writes in real-time (eliminates polling latency) (#278) Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENTS.md | 2 +- dev/dev-agent.sh | 2 +- dev/phase-test.sh | 57 ++++++++++++++++++++++++++++++++++++ gardener/gardener-agent.sh | 4 +-- lib/agent-session.sh | 49 +++++++++++++++++++++++++++++-- lib/hooks/on-phase-change.sh | 24 +++++++++++++++ 6 files changed, 132 insertions(+), 6 deletions(-) create mode 100755 lib/hooks/on-phase-change.sh diff --git a/AGENTS.md b/AGENTS.md index e0d9e12..d4cb0d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -232,7 +232,7 @@ sourced as needed. | `lib/load-project.sh` | Parses a `projects/*.toml` file into env vars (`PROJECT_NAME`, `CODEBERG_REPO`, `WOODPECKER_REPO_ID`, monitoring toggles, Matrix config, etc.). | env.sh (when `PROJECT_TOML` is set), supervisor-poll (per-project iteration) | | `lib/parse-deps.sh` | Extracts dependency issue numbers from an issue body (stdin → stdout, one number per line). Matches `## Dependencies` / `## Depends on` / `## Blocked by` sections and inline `depends on #N` patterns. Not sourced — executed via `bash lib/parse-deps.sh`. | dev-poll, supervisor-poll | | `lib/matrix_listener.sh` | Long-poll Matrix sync daemon. Dispatches thread replies to the correct agent via well-known files (`/tmp/{agent}-escalation-reply`). Handles supervisor, gardener, dev, review, vault, and action reply routing. Run as systemd service. | Standalone daemon | -| `lib/agent-session.sh` | Shared tmux + Claude session helpers: `create_agent_session()`, `inject_formula()`, `agent_wait_for_claude_ready()`, `agent_inject_into_session()`, `agent_kill_session()`, `monitor_phase_loop()`, `read_phase()`. `monitor_phase_loop` sets `_MONITOR_LOOP_EXIT` to one of: `done`, `idle_timeout`, `idle_prompt` (Claude returned to `❯` for 3 consecutive polls without writing any phase — callback invoked with `PHASE:failed`, session already dead), `crashed`, or a `PHASE:*` string. Agents must handle `idle_prompt` in both their callback and their post-loop exit handler. | dev-agent.sh, gardener-agent.sh | +| `lib/agent-session.sh` | Shared tmux + Claude session helpers: `create_agent_session()`, `inject_formula()`, `agent_wait_for_claude_ready()`, `agent_inject_into_session()`, `agent_kill_session()`, `monitor_phase_loop()`, `read_phase()`. `create_agent_session(session, workdir, [phase_file])` optionally installs a PostToolUse hook (matcher `Bash\|Write`) that detects phase file writes in real-time — when Claude writes to the phase file, the hook writes a marker so `monitor_phase_loop` reacts on the next poll instead of waiting for mtime changes. `monitor_phase_loop` sets `_MONITOR_LOOP_EXIT` to one of: `done`, `idle_timeout`, `idle_prompt` (Claude returned to `❯` for 3 consecutive polls without writing any phase — callback invoked with `PHASE:failed`, session already dead), `crashed`, or a `PHASE:*` string. Agents must handle `idle_prompt` in both their callback and their post-loop exit handler. | dev-agent.sh, gardener-agent.sh | --- diff --git a/dev/dev-agent.sh b/dev/dev-agent.sh index aee14ee..84fd345 100755 --- a/dev/dev-agent.sh +++ b/dev/dev-agent.sh @@ -681,7 +681,7 @@ fi # ============================================================================= status "creating tmux session: ${SESSION_NAME}" -if ! create_agent_session "${SESSION_NAME}" "${WORKTREE}"; then +if ! create_agent_session "${SESSION_NAME}" "${WORKTREE}" "${PHASE_FILE}"; then log "ERROR: failed to create agent session" cleanup_labels cleanup_worktree diff --git a/dev/phase-test.sh b/dev/phase-test.sh index 5fcffac..3907ee6 100755 --- a/dev/phase-test.sh +++ b/dev/phase-test.sh @@ -167,6 +167,63 @@ else fail "needs_human mtime guard: expected 1 notify call, got $NOTIFY_COUNT" fi +# ── Test 9: PostToolUse hook writes marker on phase file reference ──────── +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" + if [ -f "$MARKER_FILE" ]; then + ok "PostToolUse hook writes marker when phase file referenced" + else + fail "PostToolUse hook did not write marker" + 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" + 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" +else + fail "PostToolUse hook script not found or not executable: $HOOK_SCRIPT" +fi + +# ── Test 10: phase-changed marker resets mtime guard ───────────────────── +# Simulates monitor_phase_loop behavior: when marker exists, last_mtime +# is reset to 0 so the phase is processed even if mtime hasn't changed. +echo "PHASE:awaiting_ci" > "$PHASE_FILE" +LAST_MTIME=$(stat -c %Y "$PHASE_FILE" 2>/dev/null || echo 0) +PHASE_MTIME="$LAST_MTIME" + +# Without marker, mtime guard blocks processing (same mtime) +if [ "$PHASE_MTIME" -le "$LAST_MTIME" ]; then + ok "mtime guard blocks when no marker present (baseline)" +else + fail "mtime guard should block when phase_mtime <= last_mtime" +fi + +# Now simulate marker present — reset last_mtime to 0 +MARKER_FILE="/tmp/phase-changed-test-session.marker" +date +%s > "$MARKER_FILE" +if [ -f "$MARKER_FILE" ]; then + rm -f "$MARKER_FILE" + LAST_MTIME=0 +fi + +if [ "$PHASE_MTIME" -gt "$LAST_MTIME" ]; then + ok "phase-changed marker resets mtime guard (phase now processable)" +else + fail "phase-changed marker did not reset mtime guard" +fi + # ── Cleanup ─────────────────────────────────────────────────────────────────── rm -f "$PHASE_FILE" diff --git a/gardener/gardener-agent.sh b/gardener/gardener-agent.sh index 33ad66b..3a3f658 100644 --- a/gardener/gardener-agent.sh +++ b/gardener/gardener-agent.sh @@ -264,7 +264,7 @@ touch "$RESULT_FILE" # ── Create tmux session ─────────────────────────────────────────────────── log "Creating tmux session: ${SESSION_NAME}" -if ! create_agent_session "$SESSION_NAME" "$PROJECT_REPO_ROOT"; then +if ! create_agent_session "$SESSION_NAME" "$PROJECT_REPO_ROOT" "$PHASE_FILE"; then log "ERROR: failed to create tmux session ${SESSION_NAME}" exit 1 fi @@ -290,7 +290,7 @@ gardener_phase_callback() { log "WARNING: tmux session died unexpectedly — attempting recovery" rm -f "$RESULT_FILE" touch "$RESULT_FILE" - if create_agent_session "${_MONITOR_SESSION:-$SESSION_NAME}" "$PROJECT_REPO_ROOT" 2>/dev/null; then + if create_agent_session "${_MONITOR_SESSION:-$SESSION_NAME}" "$PROJECT_REPO_ROOT" "$PHASE_FILE" 2>/dev/null; then agent_inject_into_session "${_MONITOR_SESSION:-$SESSION_NAME}" "$PROMPT" log "Recovery session started" else diff --git a/lib/agent-session.sh b/lib/agent-session.sh index 2953802..b9b8f7f 100644 --- a/lib/agent-session.sh +++ b/lib/agent-session.sh @@ -45,10 +45,17 @@ agent_inject_into_session() { # Create a tmux session running Claude in the given workdir. # Installs a Stop hook for idle detection (see monitor_phase_loop). +# Optionally installs a PostToolUse hook for phase file write detection. +# Args: session workdir [phase_file] # Returns 0 if session is ready, 1 otherwise. create_agent_session() { local session="$1" local workdir="${2:-.}" + local phase_file="${3:-}" + + # Prepare settings directory for hooks + mkdir -p "${workdir}/.claude" + local settings="${workdir}/.claude/settings.json" # Install Stop hook for idle detection: when Claude finishes a response, # the hook writes a timestamp to a marker file. monitor_phase_loop checks @@ -56,8 +63,6 @@ create_agent_session() { local idle_marker="/tmp/claude-idle-${session}.ts" local hook_script="${FACTORY_ROOT}/lib/hooks/on-idle-stop.sh" if [ -x "$hook_script" ]; then - mkdir -p "${workdir}/.claude" - local settings="${workdir}/.claude/settings.json" local hook_cmd="${hook_script} ${idle_marker}" if [ -f "$settings" ]; then # Append our Stop hook to existing project settings @@ -79,6 +84,36 @@ create_agent_session() { fi fi + # Install PostToolUse hook for phase file write detection: when Claude + # writes to the phase file via Bash or Write, the hook writes a marker + # so monitor_phase_loop can react immediately instead of waiting for + # the next mtime-based poll cycle. + if [ -n "$phase_file" ]; then + local phase_marker="/tmp/phase-changed-${session}.marker" + local phase_hook_script="${FACTORY_ROOT}/lib/hooks/on-phase-change.sh" + if [ -x "$phase_hook_script" ]; then + local phase_hook_cmd="${phase_hook_script} ${phase_file} ${phase_marker}" + if [ -f "$settings" ]; then + jq --arg cmd "$phase_hook_cmd" ' + .hooks.PostToolUse = (.hooks.PostToolUse // []) + [{ + matcher: "Bash|Write", + hooks: [{type: "command", command: $cmd}] + }] + ' "$settings" > "${settings}.tmp" && mv "${settings}.tmp" "$settings" + else + jq -n --arg cmd "$phase_hook_cmd" '{ + hooks: { + PostToolUse: [{ + matcher: "Bash|Write", + hooks: [{type: "command", command: $cmd}] + }] + } + }' > "$settings" + fi + fi + rm -f "$phase_marker" + fi + rm -f "$idle_marker" tmux new-session -d -s "$session" -c "$workdir" \ "claude --dangerously-skip-permissions" 2>/dev/null @@ -145,6 +180,15 @@ monitor_phase_loop() { esac fi + # Check phase-changed marker from PostToolUse hook — if present, the hook + # detected a phase file write so we reset last_mtime to force processing + # this cycle instead of waiting for the next mtime change. + local phase_marker="/tmp/phase-changed-${_session}.marker" + if [ -f "$phase_marker" ]; then + rm -f "$phase_marker" + last_mtime=0 + fi + # Check phase file for changes local phase_mtime phase_mtime=$(stat -c %Y "$phase_file" 2>/dev/null || echo 0) @@ -217,6 +261,7 @@ agent_kill_session() { local session="${1:-}" [ -n "$session" ] && tmux kill-session -t "$session" 2>/dev/null || true rm -f "/tmp/claude-idle-${session}.ts" + rm -f "/tmp/phase-changed-${session}.marker" } # Read the current phase from a phase file, stripped of whitespace. diff --git a/lib/hooks/on-phase-change.sh b/lib/hooks/on-phase-change.sh new file mode 100755 index 0000000..b402d01 --- /dev/null +++ b/lib/hooks/on-phase-change.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# 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. +# +# Usage (in .claude/settings.json): +# {"type": "command", "command": "this-script /path/to/phase-file /path/to/marker"} +# +# Args: $1 = phase file path, $2 = marker file path + +phase_file="${1:-}" +marker_file="${2:-}" + +input=$(cat) # consume hook JSON from stdin + +[ -z "$phase_file" ] || [ -z "$marker_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