feat(hooks): maintain the RFC #122 wake pane mapping and idle gate - #338
Conversation
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""' 2>/dev/null) || exit 0 | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| agent-event-bus-cli wake-state "$STATE" --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] The busy marker is only cleared by Stop, but Stop does not run when a turn ends via user interrupt (Esc / Ctrl-C) — Claude Code documents Stop as not firing when the stoppage is a user interrupt. SessionStart clears it, but that only runs on a new/resumed/cleared session, and session-end.sh only on exit. So after an interrupt the marker stays busy until the user completes another turn by hand — which is precisely the state the wake path exists to escape.
Fails when: user submits a prompt (marker becomes busy), presses Esc mid-turn, and walks away → no Stop fires, marker stays busy indefinitely → the bridge treats the session as mid-turn forever and never injects, so every directed event for that session silently never wakes it.
This is the more damaging direction of the two. The documented known gap (idle-marked while a blocked Stop hook continues the turn) is benign because the TUI queues text; this one disables the feature outright, and self-heals only via a human typing — the thing the wake is meant to replace.
Worth considering: a freshness bound on the marker (bridge treats a busy marker older than N minutes as stale), or clearing it from the Notification hook on the idle-waiting-for-input notification, which is the one signal that fires precisely in this window.
| # no turn is in flight, and a marker orphaned by a hard kill would otherwise | ||
| # keep this session's id gated as busy. | ||
| agent-event-bus-cli panes set --session-id "$SESSION_ID" >/dev/null 2>&1 || true | ||
| agent-event-bus-cli wake-state idle --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] The comment above this line — "reaching SessionStart means no turn is in flight" — does not hold for source=compact. SessionStart fires with that source on auto-compaction, which triggers when the context fills during a long tool-heavy turn; the turn then continues with the compacted context. This hook already knows about that case (it branches on $SOURCE == "compact" at line 158), but this wake-state idle write is unconditional.
Fails when: a long turn auto-compacts → session-start.sh runs mid-turn with source=compact → marker flips to idle while the turn is still executing → the bridge types its wake prompt plus a newline into the live pane, and if a permission dialog happens to be up, the newline answers a prompt nobody saw — the exact case the PR rationale cites as the reason to gate at all.
Gating the stale-marker clear on [[ "$SOURCE" != "compact" ]] keeps the hard-kill cleanup (startup/resume/clear) while dropping the one source that is not a turn boundary. panes set on line 110 is fine unconditionally — it is idempotent and the pane has not changed.
| # Also clear any turn-state marker left behind: reaching SessionStart means | ||
| # no turn is in flight, and a marker orphaned by a hard kill would otherwise | ||
| # keep this session's id gated as busy. | ||
| agent-event-bus-cli panes set --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Suggestion] These two calls sit after the exit 0 on line 90, so they are skipped whenever bus registration fails (bus down, Tailscale flapping), and they key on $SESSION_ID from the register response rather than $CLIENT_ID from stdin — while session-end.sh and wake-state.sh both key on the stdin id and both run regardless of bus reachability.
Two consequences worth weighing:
- session-end.sh:45-57 argues explicitly that these are local filesystem operations that "must happen even when the bus is unreachable", and the stale-pane eviction described in the comment above is the cleanup for the visible-blast-radius case. That eviction is skipped exactly on the flaky-bus start where a prior session may well have died hard.
- The
SESSION_ID == CLIENT_IDinvariant is asserted fromserver.py:313in another repo. If stdin ever lackssession_id, line 74 omits--client-id, the server mints its own id, and the pane mapping lands under an id that wake-state.sh (which reads stdin) will never write a marker for — a mapped pane with no marker reads as permanently idle.
Using $CLIENT_ID and moving both calls above the registration block would make the three hooks agree on both the id source and the "local ops are unconditional" rule.
| # Consume stdin (required for hooks — must be before any exit) | ||
| INPUT=$(cat) | ||
|
|
||
| STATE="${1:-idle}" |
There was a problem hiding this comment.
[Suggestion] The default here and the *) branch below disagree about what a misconfiguration means: an unrecognized argument correctly writes nothing (and a test pins that), but a missing argument silently resolves to idle — the gate-open state. If a future settings.json edit drops the busy/idle argument from the UserPromptSubmit registration, every prompt would mark the session idle and the gate would be inert with no error anywhere. Treating an empty $1 the same as an unknown one (STATE="${1:-}", letting it fall through to *)) makes both misconfigurations fail closed.
| # simulated: the hooks' job is to invoke these with the right session | ||
| # id at the right lifecycle point, and the CLI's own suite already | ||
| # covers what the commands then write. | ||
| echo "$*" >> "$(dirname "$0")/../cli-calls.log" |
There was a problem hiding this comment.
[Suggestion] Recording rather than simulating is the right call here, and the assertions do pin the lifecycle points well. The residual risk the mock cannot cover is signature drift: agent-event-bus#149 is still open, so no released CLI has panes/wake-state yet, and every call site swallows errors with || true. A mismatch in subcommand or flag naming against the real CLI would surface as nothing at all, in either direction — never marked busy (mid-turn injection) or never marked idle (no wakes ever).
A line in the PR body recording one live end-to-end run once #149 lands — session starts, mapping appears, marker flips across a turn boundary, a DM actually wakes the pane — would close the gap the mock leaves open by design. Also worth a case here: source=compact must not write wake-state idle (see the comment on session-start.sh:111).
There was a problem hiding this comment.
Code Review — Round 1
Summary
The hook wiring, the delegation of the flock/atomic-rename contract to the CLI, the ordering rationale in session-end.sh, and the degradation paths are all well argued and well tested. Two lifecycle events break the marker's central assumption that UserPromptSubmit and Stop bracket every turn: an interrupted turn never reaches Stop (marker latches busy), and auto-compaction fires SessionStart mid-turn (marker flips to idle while a turn is in flight). Each produces one of the two outcomes the gate exists to prevent, and both are silent by construction.
Findings
- [Important]
wake-state.sh:49—busylatches after a user interrupt; the session becomes unwakeable exactly while unattended. - [Important]
session-start.sh:111— the unconditionalwake-state idlemarks the session idle onsource=compact, which occurs mid-turn. - [Suggestion]
session-start.sh:110— pane mapping is keyed on the register-response id and gated behind successful registration, unlike the other two hooks. - [Suggestion]
wake-state.sh:27— a missing argument fails open while an unknown one fails closed. - [Suggestion]
tests/test-hooks.sh:272— no live end-to-end evidence yet; agent-event-bus#149 is still open, so the CLI signature is unverified against a real implementation.
Verdict
REQUEST_CHANGES - The idle gate can be latched shut by an interrupted turn (no wake ever delivered to that session) and opened mid-turn by auto-compaction (injection into a live turn — the case this PR is written to avoid).
Automated review by Claude Code
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""' 2>/dev/null) || exit 0 | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| agent-event-bus-cli wake-state "$STATE" --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] (carried from round 1 — still unaddressed, no decline recorded)
busy is written at UserPromptSubmit and cleared only at Stop, but Stop does not fire when a turn ends via user interrupt (Esc / Ctrl-C) — Claude Code documents it as not running when the stoppage is a user interrupt. The other two clear points do not help: SessionStart only runs on a new/resumed/cleared session, and session-end.sh only on exit. So the marker latches.
Fails when: user presses Esc mid-turn and walks away → Stop never runs → the busy marker stays set → the bridge treats the session as mid-turn indefinitely and never delivers a wake, precisely in the unattended-idle state the bridge exists to serve.
A hook cannot observe an interrupt, so the fix has to come from outside this script. Two options worth weighing:
- Age the marker out on the reader side — have the bridge treat a
busymarker older than some threshold (mtime-based) as stale. That also covers the documented Stop-hook-blocks gap, in the other direction. - Add a
Notificationregistration runningwake-state.sh idle— that hook fires when prompt input has gone idle, which is exactly the post-interrupt state.
| # the failure it would leave — a resumed session gated busy while idle — is | ||
| # invisible from both sides. | ||
| agent-event-bus-cli panes set --session-id "$SESSION_ID" >/dev/null 2>&1 || true | ||
| agent-event-bus-cli wake-state idle --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] (carried from round 1 — the comment added this round explains why the call stays, but not why it is safe on source=compact)
The comment at line 107 — "reaching SessionStart means no turn is in flight" — does not hold for source=compact. Auto-compaction fires SessionStart with that source when the context fills during a long tool-heavy turn, and the turn then continues with the compacted context. The script already knows about that case: it parses $SOURCE at line 44 and branches on compact at line 164.
Fails when: auto-compaction triggers mid-turn → SessionStart runs with source=compact → the busy marker is cleared while the turn is still executing → the bridge types its wake prompt plus a newline into a live pane, which is both the mid-turn injection this PR is written to avoid and the one window where a permission dialog can be on screen to consume that newline.
The commit pushed this round widens it: it documents that panes set clears the marker too, so a fix that guards only line 117 leaves the window open. Both calls need the guard — e.g. wrap them in if [[ "$SOURCE" != "compact" ]]. Skipping panes set on compact costs nothing, since the mapping was already written at the real session start and the pane has not changed.
| export PATH="$HOME/.antigravity-ide/antigravity-ide/bin:$PATH" | ||
|
|
||
| # Added by Antigravity IDE | ||
| export PATH="/Users/evansenter/.antigravity-ide/antigravity-ide/bin:$PATH" |
There was a problem hiding this comment.
[Suggestion] This looks like an accidental inclusion — the Antigravity IDE installer re-appended its export and it rode along in the docs commit. Three reasons to drop it:
- It duplicates line 143, which already puts the same directory on
PATH. - It hardcodes
/Users/evansenter, so on the Debian/SteamOS targets this repo supports it prepends a path that cannot exist. The line above deliberately uses$HOME. - The comment on line 142 records that the installer output was curated once already ("installer's ~/.local/bin export omitted"); this re-append partly undoes that.
Unrelated to the wake-mapping work either way, so it belongs in its own commit if it is intentional.
There was a problem hiding this comment.
Code Review — Round 2
Summary
The only code change since round 1 is a comment on session-start.sh explaining why the wake-state idle call is kept, plus a stray .zshrc line the Antigravity installer re-appended. Neither round-1 blocking finding is addressed, and no "Feedback Addressed" comment declines them — so both still stand.
The new comment sharpens the compaction finding rather than resolving it: it documents that panes set clears the marker too, so a guard on the wake-state idle line alone would leave that window open.
Previously Addressed (Filtered)
None — there is no "Feedback Addressed" comment on this PR.
Round-1 suggestions still open, not re-posted inline to avoid duplicate threads: wake-state.sh:27 (a missing argument fails open to idle while an unknown one fails closed), session-start.sh:116 (mapping is gated behind successful registration and keyed on the register-response id, unlike the other two hooks), tests/test-hooks.sh:272 (agent-event-bus#149 is still open, so the panes / wake-state signatures are unverified against a real CLI).
Findings
- [Important]
wake-state.sh:49—busylatches after a user interrupt; that session is never wakeable again. - [Important]
session-start.sh:117— the unconditional clear marks the session idle onsource=compact, which fires mid-turn;panes seton the line above clears it too, so both calls need the guard. - [Suggestion]
.zshrc:146— stray installer re-append: duplicates line 143, hardcodes/Users/evansenter, unrelated to this PR.
Verdict
REQUEST_CHANGES - The idle gate still latches shut after a user interrupt (Stop never fires, so that session is never wakeable again) and still opens mid-turn on auto-compaction (SessionStart with source=compact clears the busy marker while the turn continues). Those are the two outcomes the gate exists to prevent, and both are silent by construction.
Automated review by Claude Code
|
Follow-up from the paired review on evansenter/agent-event-bus#149: |
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""' 2>/dev/null) || exit 0 | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| agent-event-bus-cli wake-state "$STATE" --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] (carried from rounds 1–2 — still unaddressed, no decline recorded)
busy is written at UserPromptSubmit and cleared only at Stop, but Stop does not fire when a turn ends via user interrupt (Esc / Ctrl-C) — Claude Code documents it as not running when the stoppage is a user interrupt. The other clear points do not cover it: SessionStart only runs on a new/resumed/cleared session (and, as of this round, is deliberately inert on compact), and session-end.sh only on exit. So the marker latches until a human completes another turn by hand.
Fails when: user presses Esc mid-turn and walks away → Stop never runs → the busy marker stays set → the bridge treats the session as mid-turn indefinitely and never injects, so every directed event for that session silently fails to wake it.
This is the more damaging of the two directions. The documented known gap (idle-marked while a blocked Stop hook continues the turn) is benign because the TUI queues text; this one disables the feature outright, and self-heals only via the human typing that the wake exists to replace.
The fix has to come from outside this script:
- Age the marker out on the reader side (bridge treats a
busymarker older than N minutes as stale). agent-event-bus#149 argues against a TTL, but its self-healing cases — hard kill, crash, reboot, timed-out Stop hook — do not include an interrupted turn on a still-live session, so that rationale does not cover this. - Register
wake-state.sh idleon theNotificationhook, which fires when prompt input has gone idle — exactly the post-interrupt state. - Or record it as an accepted limitation alongside the three already listed in docs: Update event bus documentation for broadcast model #149's "Not solved" section, which would close this out as a Skipped item.
Worth checking alongside this: whether UserPromptSubmit fires for a manual /compact. If it does, this round's (correct) decision to skip the clear on source=compact adds a second path to the same latched-busy state.
| # (injected text plus a newline answering a permission dialog nobody saw). | ||
| # Only the hook knows the source, so only the hook can make this call. | ||
| PANES_ARGS=(--session-id "$SESSION_ID") | ||
| [[ "$SOURCE" == "compact" ]] && PANES_ARGS+=(--keep-wake-state) |
There was a problem hiding this comment.
[Suggestion] The version-skew comment at line 121 covers one direction — a CLI whose panes set predates the marker-clearing behaviour — but this line opens the other. A machine that pulls dotfiles before agent-event-bus gets a panes set that does not recognize --keep-wake-state, so on compact the whole invocation exits with an argparse error, swallowed by >/dev/null 2>&1 || true.
Benign today: the mapping was already written at the real session start and the pane has not changed, and an old CLI does not clear the marker either — so the two skew directions cancel here. Worth a sentence in the comment so the next reader does not have to re-derive that, rather than reading it as an oversight.
(#149 is still open, so this is the live state on every box until it lands.)
There was a problem hiding this comment.
Code Review — Round 3
Summary
The compaction finding from rounds 1–2 is resolved, and resolved better than suggested: panes set grew --keep-wake-state rather than being skipped outright, so the mapping is still refreshed on compact while the turn-state marker survives — and both the compact and startup paths are pinned by tests.
The other round-1 blocking finding — the busy marker latching after a user interrupt — is unchanged, with no decline recorded, so it still stands.
Previously Addressed (Filtered)
No "Feedback Addressed" comment on this PR. Filtering instead on the owner comment of 2026-08-15, which resolves the source=compact finding:
- [Important]
session-start.sh— unconditionalwake-state idleclears the marker mid-turn onsource=compact→ Implemented (--keep-wake-stateonpanes set,$SOURCE != compactguard on the explicit clear, +2 tests). Not re-raised.
Round-1 suggestions still open, not re-posted inline to avoid a third duplicate thread:
wake-state.sh:27— a missing argument resolves toidle(gate-open) while an unknown one correctly writes nothing;STATE="${1:-}"would make both misconfigurations fail closed.session-start.sh:117— the two calls sit after theexit 0on registration failure and key on the register-response$SESSION_ID, whilesession-end.shandwake-state.shboth key on the stdin id and run regardless of bus reachability.tests/test-hooks.sh:272— agent-event-bus#149 is still open, sopanes/wake-state/--keep-wake-stateremain unverified against a released CLI, and every call site swallows errors with|| true..zshrc:146— the stray Antigravity installer re-append is still in the diff: duplicates line 143, hardcodes/Users/evansenter, unrelated to this PR.
Findings
- [Important]
wake-state.sh:49—busylatches after a user interrupt; that session is never wakeable until a human completes another turn. - [Suggestion]
session-start.sh:118—--keep-wake-stateagainst a CLI that predates it fails the wholepanes setcall; benign, but the skew comment below covers only the other direction.
On the one blocking finding
It is not fixable inside wake-state.sh — a hook cannot observe an interrupt — so the resolution may well live in the paired PR, or be a deliberate accept. Worth noting that agent-event-bus#149 argues against a marker TTL, but the cases it enumerates as self-healing (hard kill, crash, reboot, timed-out Stop hook) do not include this one: an interrupted turn leaves a live session marked busy, and only a human completing another turn clears it. A Feedback Addressed → Skipped entry recording that trade-off resolves this for the next round just as well as a code change would.
Verdict
REQUEST_CHANGES - The idle gate still latches shut after a user interrupt: Stop does not fire on interrupt, so the busy marker written at UserPromptSubmit is never cleared, and the bridge never wakes that session — precisely the unattended-idle state the wake path exists to serve.
Automated review by Claude Code
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""' 2>/dev/null) || exit 0 | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| agent-event-bus-cli wake-state "$STATE" --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] The busy marker has no path back to idle when a turn ends by interrupt rather than completion. Claude Code's Stop hook does not run when the stoppage is a user interrupt, so the marker written here at UserPromptSubmit is never cleared for the rest of that session's life.
Fails when: the user presses Esc mid-turn and walks away → Stop never fires → the busy marker persists → the bridge reads the session as mid-turn indefinitely and injects nothing, so an idle, unattended session is never woken (and drain-directed-events.sh, the fallback the gate's rationale leans on, also only runs at end-of-turn).
The three clears that exist all require an event that will not arrive here: session-start.sh (needs a restart/resume), session-end.sh (needs the session to exit), and the next Stop (needs a human to submit and complete another turn).
Not fixable inside this hook — the resolution is likely a staleness rule on the bridge side (treat a busy marker older than N minutes as idle) in agent-event-bus#149, or a recorded decision to accept it. That PR argues against a marker TTL, but the cases it lists as self-healing — hard kill, crash, reboot, timed-out Stop hook — do not include this one, where the session stays alive and marked busy.
| # other entry pointing at this pane, which cleans up after a previous session | ||
| # that died without reaching this hook. | ||
| agent-event-bus-cli panes clear --session-id "$CLIENT_ID" >/dev/null 2>&1 || true | ||
| agent-event-bus-cli wake-state idle --session-id "$CLIENT_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Suggestion] The comment above says this block drops "any turn-state marker", and the PR body says session-end.sh "removes both" — but wake-state idle sets a state; only panes clear is a removal. If the CLI represents idle as a written marker rather than as the absence of the busy one, every session that ever ran leaves an idle entry behind keyed to a session id that no longer exists.
Harmless for correctness either way — panes clear on the line above already makes the session unresolvable to the bridge — but the wording and the call do not agree. If the CLI grows a wake-state clear, this is the call site for it; otherwise a word change ("reset" rather than "drop"/"remove") keeps the comment true.
There was a problem hiding this comment.
Code Review — Round 4
Summary
The only delta since round 3 is session-start.sh:121-124 switching the compact-source arg append from [[ ... ]] && PANES_ARGS+=(...) to if/fi. Correct, and the comment documents the set -e landmine it avoids — no findings against it.
The round-1 blocking finding is unchanged: the busy marker latches after a user interrupt, because Claude Code's Stop hook does not fire on interrupt. No code change, and no Feedback Addressed → Skipped entry declining it, so it still stands.
Previously Addressed (Filtered)
No "Feedback Addressed" comment on this PR. Filtering instead on the owner comment of 2026-08-15:
- [Important]
session-start.sh— unconditionalwake-state idleclears the marker mid-turn onsource=compact→ Implemented (--keep-wake-stateonpanes set,$SOURCE != compactguard on the explicit clear, +2 tests). Not re-raised.
Standing suggestions from rounds 1–3, not re-posted inline — a fourth duplicate thread adds no information:
wake-state.sh:27—STATE="${1:-idle}"makes a missing argument fail open toidlewhile an unknown one correctly writes nothing.session-start.sh:121— the two calls sit after theexit 0on registration failure and key on the register-response$SESSION_ID, whilesession-end.shandwake-state.shkey on the stdin id and run regardless of bus reachability.session-start.sh:125—--keep-wake-stateagainst a CLI that predates it fails the wholepanes setcall; the skew comment below covers only the other direction.tests/test-hooks.sh:267— agent-event-bus#149 is still open, sopanes/wake-state/--keep-wake-stateremain unverified against a released CLI, and every call site swallows errors with|| true..zshrc:146— the stray Antigravity installer re-append is still in the diff: duplicates line 143, hardcodes/Users/evansenter, unrelated to this PR.
Findings
- [Important]
wake-state.sh:49—busylatches after a user interrupt; that session is never wakeable again until a human completes another turn. - [Suggestion]
session-end.sh:57—wake-state idleon an ending session sets a state rather than removing one, while the comment above it and the PR body both say "removes both".
On the one blocking finding
It is not fixable inside wake-state.sh — a hook cannot observe an interrupt — so the resolution plausibly lives in the paired PR (a staleness rule on the marker), or is a deliberate accept. Worth noting the asymmetry: session-start.sh:127-134 exists precisely because a latched busy marker is a real hazard ("a resumed session maps its pane correctly and still sits gated busy while idle, reported by nothing"). The interrupt case is that same hazard in a live session, where no SessionStart will ever arrive to clear it.
A Feedback Addressed → Skipped entry recording the trade-off closes this for the next round exactly as well as a code change would.
Verdict
REQUEST_CHANGES - The idle gate latches shut after a user interrupt: Stop does not fire on interrupt, so the busy marker written at UserPromptSubmit is never cleared and the bridge never wakes that session — precisely the unattended-idle state the wake path exists to serve.
Automated review by Claude Code
The agent-event-bus bridge can wake an idle session by typing a prompt into its terminal pane, but nothing has ever written the mapping that tells it which pane - so the wake path has never run. These hooks write it. session-start.sh maps the session to its pane; session-end.sh removes it. Both go through `agent-event-bus-cli panes`, which implements the atomic write + flock contract from docs/BRIDGE.md and is tested against the bridge's own reader. Doing it in bash was the obvious alternative and does not work: macOS ships no flock(1), and concurrent SessionStart hooks silently lose each other's entries with no error on either side. session-end.sh clears BEFORE unregistering, because a stale mapping is the one failure here with a visible blast radius - the bridge would type its wake prompt into whatever now owns the pane - and that cleanup must happen even when the bus is unreachable. wake-state.sh is the idle gate: busy at UserPromptSubmit, idle at Stop. The bridge injects only between turns, which costs no coverage (mid-turn, drain-directed-events.sh already surfaces directed events at end-of-turn) and keeps keystrokes out of the one window where a permission dialog can consume them. Registered last in Stop - it never blocks, so its position cannot affect which block decision wins. The session id comes from stdin rather than the bus: Claude Code's session_id IS the bus session_id, since registration passes it as --client-id and the bus adopts a client_id verbatim. So the turn-boundary hook never waits on the network. Every path exits 0. No CLI, no jq, no session_id, or a bad state argument all mean "no idle gate", never a failed turn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`panes set` clears the turn-state marker itself as of agent-event-bus#149, so this call is redundant against a current CLI. It stays because the two repos version independently: a machine that pulls dotfiles before agent-event-bus has a `panes set` that does not clear, and the failure that would leave - a resumed session gated busy while idle - is invisible from both sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion SessionStart is not only a between-turns event: an auto-compaction fires it with source=compact in the MIDDLE of a long turn. Clearing the turn-state marker there leaves the session reading idle for the rest of that turn, reopening exactly the window the idle gate exists to close — a wake landing where a permission dialog could consume it. Only the hook knows the source, so only the hook can make this call: `panes set` takes --keep-wake-state for that case (agent-event-bus#149), and the redundant `wake-state idle` call is skipped for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mid-script under `set -euo pipefail` the `[[ ]] && ARGS+=(...)` form does not exit — verified, and the startup-path test proves the hook was fine as written. As the last statement of a function or script it returns 1 and would exit the hook before `panes set` ran, silently leaving the pane unmapped. Not worth leaving that for whoever moves the line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5e397c4 to
78567fa
Compare
| SESSION_ID=$(echo "$INPUT" | jq -r '.session_id // ""' 2>/dev/null) || exit 0 | ||
| [[ -z "$SESSION_ID" ]] && exit 0 | ||
|
|
||
| agent-event-bus-cli wake-state "$STATE" --session-id "$SESSION_ID" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
[Important] The busy marker latches after a user interrupt, leaving the session permanently unwakeable.
UserPromptSubmit writes busy here; only Stop writes idle. The Stop hook in Claude Code does not run when a turn ends via user interrupt (Esc), so nothing clears the marker. Unlike the hard-kill case, no SessionStart follows to clear it either — the session is still live — so it stays busy until a human submits another prompt AND lets that turn finish.
Fails when: user presses Esc mid-turn and walks away → no Stop fires → the <session_id>.busy marker is never cleared → the bridge skips this session on every wake attempt and directed events for it are never delivered, which is exactly the unattended-idle state RFC #122 exists to serve.
Not fixable inside this hook — a hook cannot observe an interrupt. Plausible resolutions: a staleness/TTL rule on the marker in the paired CLI, or a documented accept. A Feedback Addressed → Skipped entry recording the trade-off closes this for the next round just as well as a code change would.
There was a problem hiding this comment.
Code Review — Round 5
Summary
The head commit is unchanged since round 4 (78567fa), so there is no new surface to review and no new findings. The one blocking item from round 1 stands: UserPromptSubmit writes busy and only Stop clears it, and Stop does not fire on a user interrupt — so an interrupted turn leaves a live session latched busy and never wakeable. No code change, and no Feedback Addressed → Skipped entry declining it, so it is not filtered.
Previously Addressed (Filtered)
No Feedback Addressed comment on this PR. Filtering instead on the owner comment of 2026-08-15:
- [Important]
session-start.sh— unconditionalwake-state idlecleared the marker mid-turn onsource=compact→ Implemented (--keep-wake-stateonpanes set,$SOURCE != compactguard on the explicit clear, +2 tests). Not re-raised.
Standing suggestions from rounds 1–4, not re-posted inline — a fifth duplicate thread adds nothing:
wake-state.sh:27—STATE="${1:-idle}"makes a missing argument fail open toidle, while an unknown one correctly writes nothing.session-start.sh:121— both calls sit after theexit 0on registration failure and key on the register-response$SESSION_ID, whilesession-end.shandwake-state.shkey on the stdin id and run regardless of bus reachability.session-start.sh:125—--keep-wake-stateagainst a CLI that predates it fails the wholepanes setcall; the skew comment below covers only the other direction.session-end.sh:57—wake-state idlesets a state on an ending session rather than removing one, while the comment above it and the PR body both say "removes both".tests/test-hooks.sh:267— agent-event-bus#149 is still open, sopanes/wake-state/--keep-wake-stateremain unverified against a released CLI, and every call site swallows errors with|| true..zshrc:146— the stray Antigravity installer re-append is still in the diff: duplicates line 143, hardcodes/Users/evansenter, unrelated to this PR.
On the one blocking finding
It is not fixable inside wake-state.sh — a hook cannot observe an interrupt — so the resolution plausibly lives in the paired PR (a staleness rule on the marker) or is a deliberate accept. The asymmetry is worth restating: session-start.sh:127-134 exists because a latched busy marker is a real hazard ("a resumed session maps its pane correctly and still sits gated busy while idle, reported by nothing"). The interrupt case is that same hazard in a live session, where no SessionStart will ever arrive to clear it — and it is not among the self-healing cases agent-event-bus#149 enumerates (hard kill, crash, reboot, timed-out Stop hook).
A Feedback Addressed → Skipped entry recording the trade-off closes this out for the next round exactly as well as a code change would.
Verdict
REQUEST_CHANGES - The idle gate latches shut after a user interrupt: Stop does not fire on interrupt, so the busy marker written at UserPromptSubmit is never cleared and the bridge never wakes that session — precisely the unattended-idle state the wake path exists to serve.
Automated review by Claude Code
Two standing review findings on #338. The mapping and turn-state calls sat after the `exit 0` on registration failure and keyed on the register response, so a session started while the bus was unreachable was never mapped - and therefore never wakeable - for its entire life, with the bus coming back changing nothing until the next restart. They are local filesystem writes and now run before registering, keyed on the stdin session id. That is sound because the two are the same value by construction (registration passes --client-id and the bus adopts it verbatim), and it matches session-end.sh and wake-state.sh, which already key on the stdin id. wake-state.sh no longer defaults a MISSING argument to `idle`. zj-status.sh and tmux-status.sh use `${1:-waiting}` and that is fine for a status bar, but this argument decides whether a wake may be injected: defaulting a missing one to idle fails OPEN, so a miswiring would silently disable the gate and let a wake land mid-turn. A missing argument is not evidence of either state, so it is now treated like an unknown one - write nothing, leave the last known state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Feedback Addressed The The finding is right, and it invalidated a documented argument rather than just an implementation. Nothing more is needed on this side — Both standing suggestions taken (this push):
129 tests (+2), covering the bus-down mapping path and the missing-argument path. |
| } | ||
|
|
||
| test_wake_state_busy_invokes_cli() { | ||
| setup_mock_cli |
There was a problem hiding this comment.
[Suggestion] setup_mock_cli is not defined anywhere in the repo — a repo-wide grep for its definition returns nothing. The helper that writes the mock CLI into the test bin is setup_mock_event_bus_cli (line 148), which every pre-existing test calls by that name.
The twelve new tests survive the typo by accident: run_test evaluates the body inside an if condition, so errexit is suspended and the 127 from the not-found command is swallowed (its stderr is eaten by the 2>/dev/null on that eval), while the mock itself happens to already be on PATH from the session-start.sh group that runs earlier in main(). They pass, but none of them sets up its own fixture.
Consequence: run the wake-state group standalone, or move it ahead of the === session-start.sh === block in main(), and command -v agent-event-bus-cli fails inside every hook under test — the assertions stop exercising anything. Renaming all twelve call sites to setup_mock_event_bus_cli fixes this and the leak flagged below.
| panes | wake-state) echo "$*" >> "$(dirname "$0")/../cli-calls.log"; echo '{"ok":true}' ;; | ||
| *) exit 1 ;; | ||
| esac | ||
| MOCK_DOWN |
There was a problem hiding this comment.
[Suggestion] This mock overwrites the CLI in the test bin for the rest of the run, and nothing restores it — the setup_mock_cli call at the top of each following test is a no-op (see the comment above).
The four tests that run next in main() — clears stale busy marker, (compact) preserves turn state, (startup) clears turn state, session-end clears the mapping — therefore all execute against this crippled stub, whose register fails and whose catch-all branch exits 1. They still pass, because they only assert on the panes / wake-state lines the stub also logs, but they are not testing what their comments claim.
test_session_start_startup_clears_turn_state is the clearest case: it is written as "the contrast case: a normal start IS between turns" and is in fact a second run of the bus-down path. Fixing the helper name restores the healthy mock at the start of each of those tests and makes the contrast real.
There was a problem hiding this comment.
Code Review — Round 6
Summary
Both standing suggestions landed cleanly. Moving the panes set / wake-state idle calls ahead of registration and re-keying them on the stdin CLIENT_ID is the right fix — they are local filesystem writes, and gating them on the bus being reachable was the bug. Dropping the idle default in wake-state.sh closes the fail-open miswiring. Every new path respects set -euo pipefail, the [[ -z "$SESSION_ID" ]] && exit 0 line is not terminal so it cannot leak a non-zero exit, and wake-state.sh writes nothing to stdout — which matters for a UserPromptSubmit hook. All four CI gates are green.
The round-1/3/5 blocker (the busy marker latching after an interrupt) is filtered per the previous-feedback rule: the Feedback Addressed comment records it as resolved in agent-event-bus#149 via an mtime-refreshed marker that ages out, with wake-state.sh busy serving as the refresh call. Not re-raised.
Previously Addressed (Filtered)
- [Important]
wake-state.sh—busylatches after a user interrupt → Implemented (paired repo:set_busytouches mtime,is_busyages it out). - [Suggestion]
session-start.sh— calls sat behind registration succeeding → Implemented (moved aboveregister, keyed onCLIENT_ID, +1 test). - [Suggestion]
wake-state.sh— a missing argument defaulted toidle→ Implemented (no default, +1 test). - Still-open suggestions from rounds 1–5 — the stray
.zshrcAntigravity re-append,session-end.sh:57setting a state rather than removing one,--keep-wake-stateagainst an older CLI, and the unreleased-CLI signature risk — are deliberately not re-posted inline. A sixth duplicate thread adds no information.
Findings
Two new ones this round, both against the test scaffolding added in this push, both non-blocking and posted inline:
- [Suggestion]
tests/test-hooks.sh:1013—setup_mock_cliis not defined anywhere in the repo; the intended name issetup_mock_event_bus_cli. The twelve new tests install no fixture of their own and pass on state left behind by an earlier group. - [Suggestion]
tests/test-hooks.sh:1141— because of that, the bus-down mock is never restored, so the four tests after it silently run against the crippled stub rather than the healthy one.
Verdict
APPROVE - No blocking findings. Converged at round 6 — remaining feedback is non-blocking.
Automated review by Claude Code
Installs the session-side half of agent-event-bus RFC #122. Pairs with evansenter/agent-event-bus#149, which adds the
panes/wake-stateCLI commands these call and flips the bridge to the injecting backend.The bridge can wake an idle session by typing a prompt into its terminal pane, but nothing has ever written the mapping that tells it which pane — so the wake path has never run end-to-end.
What lands
session-start.sh— writes this session's pane mapping, and clears any turn-state marker orphaned by a hard kill.session-end.sh— removes both, before unregistering.wake-state.sh(new) —busyatUserPromptSubmit,idleatStop. The idle gate.Why the write goes through the CLI
The obvious implementation is shell in the hook, and it doesn't work. The contract in
docs/BRIDGE.mdrequires an atomic temp-file + rename and an flock across the read-modify-write, and macOS ships noflock(1). Without it, concurrent SessionStart hooks silently lose each other's entries — the loser reads as unmapped later, which is the documented normal outcome for a session on another machine, so nothing errors on either side.Putting it in the CLI also means the writer and the bridge's reader run the same validator in one test suite, so they can't drift. A drift wouldn't raise anywhere; it would just be wakes that never happen.
Ordering details that are load-bearing
session-end.shclears before unregistering. A stale mapping is the one failure here with a visible blast radius — the bridge would type its wake prompt into whatever now owns the pane, usually the shell left behind — and that cleanup must happen even when the bus is unreachable.wake-state.shis registered last inStop, afterenforce-insight-publish.sh. It never blocks, so its position can't affect which block decision wins, which keeps it clear of the documented load-bearing ordering betweendrain-directed-events.shandenforce-insight-publish.sh.Why gate on idle at all
Mid-turn injection is redundant —
drain-directed-events.shalready surfaces directed events at end-of-turn — and it's the one window where a permission dialog can be on screen for injected text plus a newline to answer a prompt nobody saw. Gating loses no coverage and removes that case.Known gap, documented in the hook and the README: when a Stop hook blocks, the turn continues but this hook has already marked the session idle, so a wake in that window lands mid-turn. Benign — the TUI queues typed text — and unavoidable, since a hook can't observe another hook's block decision.
Session id without a round-trip
Straight from stdin. Claude Code's
session_idis the bussession_id:session-start.shregisters with--client-idset to it, and the bus adopts aclient_idas itssession_idverbatim (server.py:313). So the turn-boundary hook never waits on the network — which matters, since it runs twice per turn.Degradation
Every path exits 0. No CLI, no
jq, nosession_id, or an unrecognized state argument all mean "no idle gate", never a failed turn. The bridge is optional and experimental; a box without it is unaffected.Tests
125 pass (
+10). The mock CLI recordspanes/wake-stateinvocations, so the tests assert the hooks call the right command with the right session id at the right lifecycle point — the CLI's own suite covers what those commands then write.shellcheckclean.🤖 Generated with Claude Code