Skip to content

feat(hooks): maintain the RFC #122 wake pane mapping and idle gate - #338

Merged
evansenter merged 5 commits into
mainfrom
feat/wake-pane-mapping
Aug 15, 2026
Merged

feat(hooks): maintain the RFC #122 wake pane mapping and idle gate#338
evansenter merged 5 commits into
mainfrom
feat/wake-pane-mapping

Conversation

@evansenter

Copy link
Copy Markdown
Owner

Installs the session-side half of agent-event-bus RFC #122. Pairs with evansenter/agent-event-bus#149, which adds the panes / wake-state CLI 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) — busy at UserPromptSubmit, idle at Stop. 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.md requires an atomic temp-file + rename and an flock across the read-modify-write, and macOS ships no flock(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.sh clears 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.sh is registered last in Stop, after enforce-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 between drain-directed-events.sh and enforce-insight-publish.sh.

Why gate on idle at all

Mid-turn injection is redundantdrain-directed-events.sh already 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_id is the bus session_id: session-start.sh registers with --client-id set to it, and the bus adopts a client_id as its session_id verbatim (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, no session_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 records panes / wake-state invocations, 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. shellcheck clean.

🤖 Generated with 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread home/.claude/hooks/session-start.sh Outdated
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread home/.claude/hooks/session-start.sh Outdated
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. 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.
  2. The SESSION_ID == CLIENT_ID invariant is asserted from server.py:313 in another repo. If stdin ever lacks session_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.

Comment thread home/.claude/hooks/wake-state.sh Outdated
# Consume stdin (required for hooks — must be before any exit)
INPUT=$(cat)

STATE="${1:-idle}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/test-hooks.sh
# 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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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:49busy latches after a user interrupt; the session becomes unwakeable exactly while unattended.
  • [Important] session-start.sh:111 — the unconditional wake-state idle marks the session idle on source=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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Age the marker out on the reader side — have the bridge treat a busy marker older than some threshold (mtime-based) as stale. That also covers the documented Stop-hook-blocks gap, in the other direction.
  2. Add a Notification registration running wake-state.sh idle — that hook fires when prompt input has gone idle, which is exactly the post-interrupt state.

Comment thread home/.claude/hooks/session-start.sh Outdated
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread home/.zshrc
export PATH="$HOME/.antigravity-ide/antigravity-ide/bin:$PATH"

# Added by Antigravity IDE
export PATH="/Users/evansenter/.antigravity-ide/antigravity-ide/bin:$PATH"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. It duplicates line 143, which already puts the same directory on PATH.
  2. 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.
  3. 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.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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:49busy latches after a user interrupt; that session is never wakeable again.
  • [Important] session-start.sh:117 — the unconditional clear marks the session idle on source=compact, which fires mid-turn; panes set on 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

@evansenter

Copy link
Copy Markdown
Owner Author

Follow-up from the paired review on evansenter/agent-event-bus#149: SessionStart fires with source=compact mid-turn after an auto-compaction, so clearing the turn-state marker there would leave the session reading idle for the rest of that turn — reopening the exact window the idle gate exists to close. Only this hook knows the source, so the source check lives here; panes set grew --keep-wake-state for it. 127 tests pass (+2 covering both the compact and startup paths).

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Age the marker out on the reader side (bridge treats a busy marker 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.
  2. Register wake-state.sh idle on the Notification hook, which fires when prompt input has gone idle — exactly the post-interrupt state.
  3. 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.

Comment thread home/.claude/hooks/session-start.sh Outdated
# (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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 — unconditional wake-state idle clears the marker mid-turn on source=compactImplemented (--keep-wake-state on panes set, $SOURCE != compact guard 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 to idle (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 the exit 0 on registration failure and key on the register-response $SESSION_ID, while session-end.sh and wake-state.sh both key on the stdin id and run regardless of bus reachability.
  • tests/test-hooks.sh:272 — agent-event-bus#149 is still open, so panes / wake-state / --keep-wake-state remain 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:49busy latches after a user interrupt; that session is never wakeable until a human completes another turn.
  • [Suggestion] session-start.sh:118--keep-wake-state against a CLI that predates it fails the whole panes set call; 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 — unconditional wake-state idle clears the marker mid-turn on source=compactImplemented (--keep-wake-state on panes set, $SOURCE != compact guard 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:27STATE="${1:-idle}" makes a missing argument fail open to idle while an unknown one correctly writes nothing.
  • session-start.sh:121 — the two calls sit after the exit 0 on registration failure and key on the register-response $SESSION_ID, while session-end.sh and wake-state.sh key on the stdin id and run regardless of bus reachability.
  • session-start.sh:125--keep-wake-state against a CLI that predates it fails the whole panes set call; the skew comment below covers only the other direction.
  • tests/test-hooks.sh:267 — agent-event-bus#149 is still open, so panes / wake-state / --keep-wake-state remain 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:49busy latches after a user interrupt; that session is never wakeable again until a human completes another turn.
  • [Suggestion] session-end.sh:57wake-state idle on 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

evansenter and others added 4 commits August 15, 2026 22:59
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>
@evansenter
evansenter force-pushed the feat/wake-pane-mapping branch from 5e397c4 to 78567fa Compare August 15, 2026 21:59
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 — unconditional wake-state idle cleared the marker mid-turn on source=compactImplemented (--keep-wake-state on panes set, $SOURCE != compact guard 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:27STATE="${1:-idle}" makes a missing argument fail open to idle, while an unknown one correctly writes nothing.
  • session-start.sh:121 — both calls sit after the exit 0 on registration failure and key on the register-response $SESSION_ID, while session-end.sh and wake-state.sh key on the stdin id and run regardless of bus reachability.
  • session-start.sh:125--keep-wake-state against a CLI that predates it fails the whole panes set call; the skew comment below covers only the other direction.
  • session-end.sh:57wake-state idle sets 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, so panes / wake-state / --keep-wake-state remain 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>
@evansenter

Copy link
Copy Markdown
Owner Author

Feedback Addressed

The busy marker latching after a user interrupt — addressed, in the other repo. This is the round-1/3/5 blocker here, and the reason it kept re-raising is that the fix isn't visible in this diff: it lives in wake.py and bridge.py on evansenter/agent-event-bus#149, which is approved and green.

The finding is right, and it invalidated a documented argument rather than just an implementation. Stop doesn't run when a turn ends by interrupt and SessionEnd doesn't fire, so the session sits idle, alive and mapped, latched busy. What closed it is a refreshed marker rather than the static TTL that design had rejected: set_busy now touches the mtime and is_busy ages it out, so a turn that keeps marking itself busy holds the gate however long it runs, while an interrupted one stops refreshing and ages out. Those two cases are exactly what the original argument conflated.

Nothing more is needed on this side — wake-state.sh busy already is the refresh call. A deployment that wants faster healing than the default hour can wire it to a per-tool-call hook, which is documented in docs/BRIDGE.md rather than imposed here, since it costs a subprocess per tool call.

Both standing suggestions taken (this push):

  • session-start.sh — the mapping and turn-state calls sat behind registration succeeding, so a session started while the bus was unreachable was never mapped and therefore never wakeable for its entire life, with the bus returning changing nothing until the next restart. They're local filesystem writes; they now run before registering, keyed on the stdin session id like session-end.sh and wake-state.sh already were. Sound because the two ids are the same value by construction.
  • wake-state.sh — a missing argument no longer defaults to idle. The ${1:-waiting} pattern I copied from zj-status.sh is fine for a status bar, but here the argument decides whether a wake may be injected, so defaulting open means a miswiring silently disables the gate. A missing argument is now treated like an unknown one: write nothing, leave the last known state.

129 tests (+2), covering the bus-down mapping path and the missing-argument path.

Comment thread tests/test-hooks.sh
}

test_wake_state_busy_invokes_cli() {
setup_mock_cli

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/test-hooks.sh
panes | wake-state) echo "$*" >> "$(dirname "$0")/../cli-calls.log"; echo '{"ok":true}' ;;
*) exit 1 ;;
esac
MOCK_DOWN

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.shbusy latches after a user interrupt → Implemented (paired repo: set_busy touches mtime, is_busy ages it out).
  • [Suggestion] session-start.sh — calls sat behind registration succeeding → Implemented (moved above register, keyed on CLIENT_ID, +1 test).
  • [Suggestion] wake-state.sh — a missing argument defaulted to idleImplemented (no default, +1 test).
  • Still-open suggestions from rounds 1–5 — the stray .zshrc Antigravity re-append, session-end.sh:57 setting a state rather than removing one, --keep-wake-state against 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:1013setup_mock_cli is not defined anywhere in the repo; the intended name is setup_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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant