feat(hooks): Stop-hook event-drain for directed events #317
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| #!/bin/bash | ||
| # Stop hook: drain DIRECTED agent-event-bus events that arrived while the | ||
| # session was working, so the agent sees DMs / help requests at end-of-turn | ||
| # instead of only on the next human prompt. | ||
| # | ||
| # Input (via stdin): JSON with session_id, stop_hook_active, cwd, transcript_path | ||
| # Output: JSON {"decision":"block","reason":"..."} when directed events wait; | ||
| # silent (exit 0) otherwise. | ||
| # | ||
| # Why this exists: | ||
| # prompt-events.sh (UserPromptSubmit) only fires when the human types. A | ||
| # directed event sent to an idle/just-finished session would otherwise sit | ||
| # unseen until the next prompt. This hook closes that gap by PEEKing the bus | ||
| # at Stop and, if anything directed is waiting, blocking once to surface it. | ||
| # | ||
| # Cursor semantics (important): | ||
| # The bus keeps ONE high-water cursor per session_id, shared by BOTH hooks. | ||
| # - PEEK (non-consuming) lets us look without moving the cursor. | ||
| # - We only CONSUME (advance the cursor) when we actually surface events. | ||
| # - When we surface, we surface ALL peeked events (directed + ambient), then | ||
| # consume, because the single cursor can't selectively skip ambient ones. | ||
| # Surfacing ambient alongside directed is lossless and correct. | ||
| # - When there are NO directed events we neither surface nor consume, leaving | ||
| # everything for prompt-events.sh to pull on the next UserPromptSubmit. | ||
| # | ||
| # Stop-hook ordering / multi-block interaction: | ||
| # Registered in settings.json AFTER enforce-insight-publish.sh. Both can emit | ||
| # {"decision":"block"}. Claude Code honors a single block decision per Stop, | ||
| # so the two can race on the same turn; that is fine - this hook only blocks | ||
| # when directed events exist, and (because it peeks, not consumes, unless it | ||
| # blocks) a turn where enforce-insight wins leaves the directed events intact | ||
| # to be drained on the next Stop. See hooks/README.md for details. | ||
|
There was a problem hiding this comment. [Important] PR body claims home/.claude/hooks/README.md was updated with hook-table rows + a new Event-drain architecture section, but the README is not in the diff. The Stop-hook lifecycle ASCII diagram (README.md:38-42) still lists only zj-status + enforce-insight-publish and the Hook Details section has no entry for drain-directed-events.sh. CLAUDE.md is explicit: when adding or modifying hooks, update the README. Per this hook own comment on line 32 (See hooks/README.md for details), the README section it points at does not exist. Please add the README updates the description references. |
||
| # | ||
| # NEVER block the session on error: every failure path exits 0. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| # Read stdin (always consume to avoid broken pipe). | ||
| INPUT=$(cat) | ||
|
|
||
| # Loop guard: do not re-fire on our own block (mirrors enforce-insight-publish.sh). | ||
| # Done before sourcing the lib / requiring the cli so the guard is robust. | ||
| if command -v jq >/dev/null 2>&1; then | ||
| STOP_ACTIVE=$(echo "$INPUT" | jq -r '.stop_hook_active // false') | ||
| [[ "$STOP_ACTIVE" == "true" ]] && exit 0 | ||
| fi | ||
|
|
||
| # Source shared collection library (denylist + fetch path shared w/ prompt-events.sh). | ||
| # shellcheck source=lib/eventbus-collect.sh | ||
| source "$(dirname "$0")/lib/eventbus-collect.sh" | ||
|
|
||
| # Graceful degradation: need cli + jq. | ||
| eb_have_deps || exit 0 | ||
|
|
||
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""') | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| # Determine this session's repo (for repo:<name> help_needed classification), | ||
| # from cwd/git like session-start.sh does. Best-effort; empty REPO is fine. | ||
| CWD=$(echo "$INPUT" | jq -r '.cwd // ""') | ||
| [[ -z "$CWD" ]] && CWD="$PWD" | ||
| REPO="" | ||
| if command -v git >/dev/null 2>&1 && git -C "$CWD" rev-parse --git-dir >/dev/null 2>&1; then | ||
| COMMON_DIR=$(git -C "$CWD" rev-parse --git-common-dir 2>/dev/null) | ||
| if [[ "$COMMON_DIR" == /* ]]; then | ||
| REPO=$(basename "$(dirname "$COMMON_DIR")") | ||
| else | ||
| REPO=$(basename "$(git -C "$CWD" rev-parse --show-toplevel 2>/dev/null)" 2>/dev/null || basename "$CWD") | ||
| fi | ||
| else | ||
| REPO=$(basename "$CWD" 2>/dev/null || echo "") | ||
| fi | ||
|
|
||
| # PEEK new events (non-consuming), JSON for classification, ascending order. | ||
| # JSON shape (verified live): {"events":[{event_id,event_type,payload,channel},...]}. | ||
| PEEKED=$(eb_fetch_events "$SESSION_ID" peek json asc) | ||
|
There was a problem hiding this comment. [Suggestion] This is the first of two peek calls per directed Stop (the second is L103 for text rendering). On every Stop where events exist, the hook makes 2x the bus traffic of a single fetch. Rendering the human text from the JSON payloads here (the payload field is already what the CLI text mode displays) would halve traffic and tighten the race window mentioned at L103. |
||
| [[ -z "$PEEKED" ]] && exit 0 | ||
|
|
||
| # Count DIRECTED events: | ||
| # DIRECTED = channel == "session:<this session_id>" | ||
| # OR (event_type == "help_needed" AND channel == "repo:<this repo>") | ||
| SESSION_CHANNEL="session:$SESSION_ID" | ||
| REPO_CHANNEL="" | ||
| [[ -n "$REPO" ]] && REPO_CHANNEL="repo:$REPO" | ||
|
|
||
| DIRECTED=$(echo "$PEEKED" | jq -r \ | ||
| --arg sc "$SESSION_CHANNEL" --arg rc "$REPO_CHANNEL" ' | ||
| (.events // []) | ||
| | [ .[] | select( | ||
| (.channel == $sc) | ||
| or (.event_type == "help_needed" and $rc != "" and .channel == $rc) | ||
| ) | ||
| ] | length' 2>/dev/null || echo 0) | ||
|
|
||
| # No directed events (or parse failure): leave everything for prompt-events.sh. | ||
| # Do NOT consume. | ||
| if [[ -z "$DIRECTED" ]] || ! [[ "$DIRECTED" =~ ^[0-9]+$ ]] || [[ "$DIRECTED" -eq 0 ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Render ALL peeked events in the CLI's own text format (same as prompt-events | ||
| # surfaces) by re-peeking as text. Still non-consuming. | ||
| SUMMARY=$(eb_fetch_events "$SESSION_ID" peek text asc) | ||
|
There was a problem hiding this comment. [Suggestion] Race window between this peek (text, for the surfaced summary) and the consume on line 112: events that arrive in the millisecond gap get consumed by L112 but were never in SUMMARY, so they are lost from Claude view. The window is small but eliminable - render the text yourself from the JSON PEEKED already in hand (line 76), and you reduce both the calls and the gap. Two birds: this race + the next comment about doubling bus traffic. |
||
| if [[ -z "$SUMMARY" || "$SUMMARY" == "No events" || "$SUMMARY" == "No new events" ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| REASON=$(printf 'Directed event(s) arrived while you were working. Address them before going idle.\n\n<recent-events>\n%s\n</recent-events>' "$SUMMARY") | ||
|
|
||
| # Consume to advance the shared cursor past what we just surfaced, so the next | ||
| # prompt-events.sh pull does not re-show these. Discard the output. | ||
| eb_fetch_events "$SESSION_ID" consume text asc >/dev/null 2>&1 || true | ||
|
There was a problem hiding this comment. [Important] Multi-hook block race can lose directed events. This hook is registered AFTER enforce-insight-publish.sh in settings.json Stop and both can emit The PR body mitigation ("a turn where enforce-insight wins leaves the directed events intact to be drained on the next Stop") only holds if Claude Code stops running subsequent Stop hooks once one returns block, OR if it surfaces both reasons. Neither is asserted in Claude Code hook contract. If both hooks run to completion and only one block decision is honored, this line consumes (cursor advances past) events whose surfaced reason was dropped - silent data loss. Safer options:
The choice deserves an explicit comment; "that is fine" in the file header is not justified by the code as written. |
||
|
|
||
| # Block once, surfacing all peeked events so the agent can act. | ||
| jq -n --arg reason "$REASON" '{decision: "block", reason: $reason}' | ||
| exit 0 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| #!/bin/bash | ||
| # Shared agent-event-bus collection library. | ||
| # | ||
| # Single source of truth for the two event-bus hooks so they CANNOT diverge: | ||
| # - prompt-events.sh (UserPromptSubmit): consumes events, injects <recent-events> | ||
| # - drain-directed-events.sh (Stop): peeks for directed events, surfaces+consumes | ||
| # | ||
| # This file is meant to be SOURCED, not executed. It provides: | ||
| # EB_URL_ARGS array of --url args (empty if AGENT_EVENT_BUS_URL unset) | ||
| # EB_EXCLUDE canonical denylist of noisy event types | ||
| # eb_have_deps func: returns 0 iff agent-event-bus-cli AND jq are present | ||
| # eb_fetch_events func: thin wrapper over `agent-event-bus-cli events` | ||
| # | ||
| # Symlink note: bootstrap symlinks the whole hooks/ dir, so lib/ rides along. | ||
| # Hooks source this via: "$(dirname "$0")/lib/eventbus-collect.sh". | ||
| # | ||
| # Callers run under `set -euo pipefail`; this lib must be safe under it. | ||
|
|
||
| # Source user's environment for AGENT_EVENT_BUS_URL, defensively (PR #314 pattern). | ||
| # ~/.extra is user-edited and untracked; a stray unset var or non-zero line must | ||
| # not abort the hook under set -euo pipefail. | ||
| if [[ -f ~/.extra ]]; then | ||
| set +eu | ||
| # shellcheck source=/dev/null | ||
| source ~/.extra | ||
| set -eu | ||
| fi | ||
|
|
||
| # Build --url args from AGENT_EVENT_BUS_URL if set (e.g. remote Tailscale endpoint). | ||
| EB_URL_ARGS=() | ||
| [[ -n "${AGENT_EVENT_BUS_URL:-}" ]] && EB_URL_ARGS=(--url "$AGENT_EVENT_BUS_URL") | ||
|
|
||
| # Canonical denylist of noisy event types. Both hooks MUST use this one list. | ||
| EB_EXCLUDE="session_registered,session_unregistered,ci_watching,task_started,ci_rerun,parallel_work_started" | ||
|
|
||
| # Graceful-degradation guard: caller should `eb_have_deps || exit 0`. | ||
| # Returns 0 iff both the CLI and jq are available. | ||
| eb_have_deps() { | ||
| command -v agent-event-bus-cli >/dev/null 2>&1 && command -v jq >/dev/null 2>&1 | ||
| } | ||
|
|
||
| # Fetch events for a session via the CLI. | ||
| # | ||
| # Usage: eb_fetch_events SESSION_ID PEEK FORMAT ORDER | ||
| # SESSION_ID required (used for cursor tracking; client-id == session-id) | ||
| # PEEK "peek" -> non-consuming (--peek); anything else -> consuming | ||
| # FORMAT "json" -> --json; anything else -> default text rendering | ||
| # ORDER "asc" | "desc" (default asc) | ||
| # | ||
| # Always uses --resume so the server tracks the per-session high-water cursor. | ||
| # JSON output is a top-level object: {"events":[{event_id,event_type,payload, | ||
| # channel}, ...], "next_cursor": ...}. Text output is the CLI's human format. | ||
| # Emits the CLI's stdout on success; empty string on any failure (never errors). | ||
| eb_fetch_events() { | ||
| local session_id="$1" peek="${2:-consume}" format="${3:-text}" order="${4:-asc}" | ||
| local flags=(--resume --session-id "$session_id" --order "$order" --exclude "$EB_EXCLUDE" --timeout 200 --limit 20) | ||
| [[ "$peek" == "peek" ]] && flags+=(--peek) | ||
| [[ "$format" == "json" ]] && flags+=(--json) | ||
| agent-event-bus-cli ${EB_URL_ARGS[@]+"${EB_URL_ARGS[@]}"} events "${flags[@]}" 2>/dev/null || true | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Important] PR body claims tests/test-hooks.sh gained 7 behavioral tests (syntax; missing cli/session_id then exit 0; loop guard; directed then block JSON; ambient-only then silent; empty then silent), but tests/test-hooks.sh is not in this PR diff and has zero references to drain-directed-events.sh or eventbus-collect.sh. Every new code path here (loop guard, dep guard, repo derivation, peek/classify branch, directed/no-directed split, consume-on-block) is untested. CLAUDE.md is explicit about new code needing tests. Please add the tests the description references (or update the description to match what shipped).