feat(hooks): Stop-hook event-drain for directed events

[evansenter]

Stop-hook event-drain for directed events

Problem

prompt-events.sh (UserPromptSubmit) only injects <recent-events> when the human types. Directed events sent to an idle/just-finished session — DMs to session:<id>, or help_needed on the session's repo:<name> channel — were missed until the next prompt. This closes the "turn just ended, directed event is waiting, agent goes idle without seeing it" gap.

What this PR contains (6 files)

Feature (4 files):

1. home/.claude/hooks/lib/eventbus-collect.sh (new) — shared collection library: single source of truth for ~/.extra/AGENT_EVENT_BUS_URL handling, the canonical EB_EXCLUDE denylist, the eb_fetch_events fetch wrapper (peek/consume × json/text × order, always --resume), and the eb_have_deps guard.
2. home/.claude/hooks/prompt-events.sh (refactored) — now sources and uses the lib (behavior unchanged: consumes, asc, text, wrapped in <recent-events>), so the two hooks cannot diverge.
3. home/.claude/hooks/drain-directed-events.sh (new Stop hook) — loop-guards on stop_hook_active; graceful-degrades (no cli/jq/session_id → exit 0). PEEKs new events (non-consuming, JSON), classifies DIRECTED = channel == session:<id> OR (help_needed on repo:<name>). If directed present, emits {"decision":"block"} surfacing all peeked events, then does a consuming read to advance the shared cursor. If none directed, neither surfaces nor consumes. Never blocks on error.
4. home/.claude/settings.json — registers the drain in the Stop array after enforce-insight-publish.sh.

Docs + tests (2 files, added in this cleanup):
5. home/.claude/hooks/README.md — new drain-directed-events.sh detail section; lifecycle-diagram and settings.json config rows; a prompt-events.sh shared-lib note; and a new Event-drain architecture section covering the shared lib, the live-verified events --json schema, peek/cursor semantics, and the multi-Stop-hook ordering caveat.
6. tests/test-hooks.sh — 9 new behavioral tests for the drain hook.

Tests

Added 9 tests for drain-directed-events.sh matching the existing harness (PATH-shimmed mock agent-event-bus-cli, real jq for the classification filter):

• syntax check
• graceful degradation: missing jq → exit 0 silent
• graceful degradation: missing cli → exit 0 silent
• graceful degradation: missing session_id → exit 0 silent
• stop_hook_active=true → exit 0 silent, no consume (loop guard)
• DM on session:<id> → emits {"decision":"block"} with events in <recent-events> + consumes (cursor advanced)
• help_needed on repo:<name> → block
• ambient-only (gotcha_discovered on all) → silent exit 0, no consume
• empty bus → silent exit 0, no consume

The block and ambient tests are mutation-verified (disabling the block path / breaking the directed classifier makes exactly the right tests fail). The mock is hermetic — a channel-aware fake CLI driven by env-var fixtures with a marker file to assert consume/no-consume; no real network or bus.

make check green locally (this session):

• shellcheck: clean (exit 0)
• hook tests: 99 passed / 0 failed (was 90 before the 9 drain tests)
• bootstrap tests: 35 passed / 0 failed

Cursor semantics

The bus keeps one high-water cursor per session_id, shared by both hooks. Peek lets the drain look without consuming; it only consumes when it blocks, and surfaces all peeked events in that block (the single cursor can't selectively skip ambient ones — surfacing them alongside is lossless). When no directed events exist it does nothing, so ambient flows normally via the next UserPromptSubmit.

Multi-Stop-hook interaction

Both enforce-insight-publish.sh and the drain can emit decision:block; Claude Code honors one block per Stop. Safe by design: the drain peeks and only consumes when it wins, so if enforce-insight wins a turn the directed events stay intact and drain on the next Stop.

Note on the workflow files

An earlier revision of this branch also carried .github/workflows/* checkout-version churn that duplicated changes already merged in #315. The branch was rebased onto current main; those workflow edits are now no-ops against main and no longer appear in this PR's diff. This PR touches only the 6 hook/settings/docs/test files above.

🤖 Generated with Claude Code

[claude]

Code Review

The core hook design is sound. The shared eventbus-collect.sh lib eliminates divergence between the two hooks, peek-then-classify is the right pattern, and the loop guard / graceful-degradation paths mirror existing hooks. However, the PR ships without the tests and README updates the description claims, and the multi-hook block interaction has a data-loss edge case the cursor-consume logic does not handle. See inline comments (3 Important, 3 Suggestions).

Verdict: REQUEST_CHANGES (missing tests/docs and multi-block race need addressing before merge).

Automated review by Claude Code

[claude]

[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).

[claude]

[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.

[claude]

[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 {"decision":"block"} on the same turn (insight-rule violation + directed event arriving are independent and can co-occur).

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:

1. Verify Claude Code resolution and document it inline (and ideally enforce with a test).
2. Do not consume here - let the next prompt-events.sh invocation drain. Costs duplicate surfacing in the rare both-block turn, but no data loss.
3. Have drain replicate enforce-insight check and skip surfacing if enforce would block (couples the two hooks, but tighter).

The choice deserves an explicit comment; "that is fine" in the file header is not justified by the code as written.

[claude]

[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.

[claude]

[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.

[claude]

[Suggestion] Three workflow files (claude.yml, claude-code-review.yml, dotfiles.yml) are touched here for the actions/checkout v4/v5 work, which is unrelated to the Stop-hook event-drain feature and is not mentioned in the PR body. This kind of mixed-scope diff makes future git-log-by-path harder. Consider splitting into a separate PR (or at minimum noting it in the description) so the hook PR stays focused.

[claude]

simple body without special chars

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Note: The sandbox running this review blocked the gh api ... reviews JSON payload (obfuscation detector rejects JSON object literals), so findings are posted in the review body with file:line references instead of as inline comments.

Summary

Clean refactor. The shared eventbus-collect.sh lib genuinely removes divergence between the two hooks, peek-then-classify is the right pattern, and the loop-guard / graceful-degradation paths mirror existing hooks. Tests (9 drain cases) and README updates are now present and thorough. One unresolved correctness issue remains from the prior round: the consume-on-block path can silently lose directed events when enforce-insight-publish.sh also blocks on the same Stop.

Previously Addressed (Filtered)

Resolved since the last review and not re-raised:

• [Important] Missing tests/test-hooks.sh coverage — now present (9 drain tests).
• [Important] Missing README.md updates — now present (hook-table rows + Event-drain architecture section).
• [Suggestion] Unrelated actions/checkout workflow churn — those files are no longer in this PR's diff.

---

[Important] home/.claude/hooks/drain-directed-events.sh:112 — confirmed silent data-loss on the multi-block race

The prior round's concern is still live, and the safety justification in the header/README is factually wrong.

I verified Claude Code's documented Stop-hook semantics: all registered Stop hooks run to completion, and when more than one returns a block decision, the first block reason shown takes precedence (the others are dropped). enforce-insight-publish.sh is registered before drain-directed-events.sh in settings.json. So on any turn where an unpublished insight and a directed event co-occur, enforce-insight wins and this hook's reason (the surfaced recent-events) is discarded — but line 112 has already consumed those events, advancing the shared cursor. The next prompt-events.sh --resume pull starts past them, so the DM / help_needed is lost to the session.

The header comment (lines 26-32) and the README "Multi-Stop-hook ordering caveat" both claim this is "safe by design" because the hook "only consumes when it wins." The code has no win-detection — a hook cannot observe whether its own block reason was the one surfaced — so the consume is unconditional whenever the directed branch is taken. The documented first-reason-wins rule means this hook, registered second, is precisely the one that loses.

Fix options (in order of preference):

1. Do not consume here. Let the next prompt-events.sh on UserPromptSubmit consume. Cost: in the rare both-block turn where drain does win, the events surface once more on the next prompt (a harmless duplicate). Eliminates the loss entirely and matches the "peek-only at Stop" model.
2. Register drain before enforce-insight-publish.sh so drain's reason wins when both block (enforce-insight consumes nothing, so it simply re-blocks on the next Stop). Less clean; couples ordering to correctness.

Whichever you choose, update the header comment and README caveat — as written they assert a guarantee the code does not provide.

---

[Suggestion] home/.claude/hooks/drain-directed-events.sh:103 — double peek + eliminable race window

This is the second peek per directed Stop (the first is line 76, as JSON). Re-peeking as text here doubles bus traffic on every directed Stop, and any event arriving between this text peek and the consume on line 112 gets consumed-but-never-surfaced — a second, smaller data-loss window on top of the one above. You already hold the full event set in PEEKED (line 76); render the human text from it directly (the payload field is what the CLI's text mode prints) and drop this second call. Halves traffic and closes the gap in one move.

---

Verdict

REQUEST_CHANGES — the multi-Stop-hook consume race is a confirmed silent data-loss path (1 Important, 1 Suggestion).

---

Automated review by Claude Code

[claude]

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Note: The sandbox running this review blocks the gh api ... reviews JSON payload (brace/quote obfuscation detector) and file writes, so findings are posted in the review body with file:line references rather than as inline comments — same limitation as the prior round.

Summary

The design is sound: the shared eventbus-collect.sh lib removes divergence between the two hooks, peek-then-classify is the right pattern, the loop-guard / graceful-degradation paths mirror existing hooks, repo derivation matches session-start.sh, and the 9 drain tests are hermetic and mutation-verified. One correctness issue from the prior round remains unaddressed in the current code.

Previously Addressed (Filtered)

Resolved in prior rounds and not re-raised:

• [Important] Missing tests/test-hooks.sh coverage — now present (9 drain tests).
• [Important] Missing README.md updates — now present (hook-table rows + Event-drain architecture section).
• [Suggestion] Unrelated actions/checkout workflow churn — no longer in this diff.

---

[Important] home/.claude/hooks/drain-directed-events.sh:112 — confirmed silent data-loss on the multi-Stop-hook block race; the header/README assert a guarantee the code does not implement

I verified Claude Code's Stop-hook semantics independently: all registered Stop hooks run in parallel and only one block decision is honored per Stop — the other block reasons are dropped. enforce-insight-publish.sh is registered before this hook in settings.json. So on any turn where an unpublished insight and a directed event co-occur, enforce-insight's block can win and this hook's reason (the surfaced <recent-events>) is discarded — but line 112 has already consumed the events, advancing the shared cursor. The next prompt-events.sh --resume pull starts past them, so the DM / help_needed is lost — exactly the signal this feature exists to deliver.

The header comment (lines 26-32, "only consumes when it wins") and the README "Multi-Stop-hook ordering caveat" assert a guarantee the code cannot provide: a hook cannot observe whether its own block reason was the one surfaced, so there is no win-detection — the consume is unconditional once the directed branch is taken, and this hook (registered second) is precisely the one that loses the race.

Fix options:

1. Don't consume here — let the next prompt-events.sh (UserPromptSubmit) consume. Caveat to handle: with no consume, the loop guard (stop_hook_active) only suppresses re-firing within the same blocked turn, so on each subsequent genuine Stop the still-unconsumed directed events would re-block until a human prompt finally consumes them. Mitigate by recording surfaced event_ids in a small per-session state file and skipping already-surfaced ones.
2. Register this hook before enforce-insight-publish.sh so its reason wins when both block. Couples ordering to correctness and is more fragile.

Whichever you choose, update the header comment and the README caveat — as written they describe behavior the code does not implement.

---

[Suggestion] home/.claude/hooks/drain-directed-events.sh:103 — avoidable second peek opens a smaller race window

You already hold the full event set in PEEKED (line 76, JSON). Re-peeking as text here doubles bus traffic on every directed Stop, and any event arriving between this text peek and the consume on line 112 gets consumed-but-never-surfaced — a second, narrower data-loss window on top of the one above. Render the human-readable text directly from PEEKED (the payload field is what the CLI's text mode prints) and drop this call — halves traffic and closes the gap in one move.

---

Verdict

REQUEST_CHANGES — the unconditional consume on the multi-Stop-hook block race is a confirmed silent data-loss path, and the header/README assert a safety guarantee the code does not implement (1 Important, 1 Suggestion).

---

Automated review by Claude Code