fix(tmux): suppress nudge Escape while agent is generating (stop interrupting the Mayor)#4242
Conversation
System nudges (witness, daemon health-check, mail, broadcast, gt nudge) deliver via tmux send-keys and, by default, send a literal Escape keystroke (step 5) to exit a vim-mode composer's INSERT mode (GH#307). In Claude Code that Escape cancels in-flight generation, so a nudge landing while an agent is mid-turn — e.g. the Mayor — interrupts its current work. Gate the Escape on the agent's busy state: skip it when the pane shows the "esc to interrupt" indicator (agent generating), send it only when idle. This reuses the same signal IsIdle/WaitForIdle already trust, rather than fragile vim-mode sniffing, and is strictly safer than the previous always-send behavior. Applied to both NudgeSessionWithOpts and NudgePane (which previously sent Escape unconditionally). Adds a pure-function test (shouldSendEscapeForLines) and a live-pane wiring test (shouldSendEscape). The "esc to interrupt" heuristic's fragility is documented in code and tracked in gastownhall#4240. Relates to gastownhall#4240 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ 12 Tests Failed:
View the full list of 12 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
(cherry picked from commit 1580c738dbd76fd223a39181009f95464db9c970)
|
PR Sheriff review complete for #4242. 15 research legs: 5 pre-implementation reviews: Fixup pushed to this PR branch:
5 post-implementation reviews: Local verification:
Recommendation: approve/merge the current fixed-up PR branch. If GitHub CI remains red with the same repo-wide baseline failures seen on |
|
CI reran after the fixup commit and remains red in unrelated baseline areas: repo-wide lint debt, convoy/session/beads runtime tests, and config expectation tests. No failure is in internal/tmux or in the new Escape-gating tests. Sheriff recommendation remains: approve/merge this fixed-up PR branch if baseline CI failures are waived or resolved separately. |
|
Restoring a comment I removed earlier by mistake. CI triage — red checks are a pre-existing baseline, not from this PRThis change touches only 1. This PR's own tests all pass in CI:
2. Every failing test/lint finding is in a package/file this PR does not touch:
3. The base commit already fails these jobs. No action needed on this PR for the red checks; they reflect existing breakage on |
|
Reviewed the maintainer fixup (
Local verification on the fixed-up branch: No further changes from my side — good to merge. The |
Problem
System nudges are delivered to agents via
tmux send-keys. The delivery protocol (NudgeSessionWithOpts/NudgePane) sends a literal Escape keystroke (step 5) before Enter — originally to exit a vim-mode composer's INSERT mode so Enter submits (GH#307), commented as "harmless for bash/Claude Code".It is not harmless for Claude Code: Escape cancels in-flight generation. So when a nudge lands while the agent is mid-turn — the Mayor being the common victim — the Escape interrupts its current work. This affects every direct
NudgeSessioncaller: witness handlers, the daemon health-check heartbeat, the mail router, broadcast, andgt nudge.EscapeCancelsRequestalready existed to skip the Escape for agents where it cancels generation (Gemini; Copilot auto-skipped), but Claude was wrongly assumed exempt.Fix
Gate the Escape on the agent's busy state instead of sending it unconditionally:
shouldSendEscape(target)captures the pane and returnsfalsewhen the busy indicator (esc to interrupt) is present — i.e. the agent is generating.!SkipEscape && shouldSendEscape(...).NudgeSessionWithOptsandNudgePane(the latter previously sent Escape with no gate at all).This reuses the exact signal
IsIdle/WaitForIdlealready trust, so it adds no new external assumption. Behavior is strictly safer:esc to interruptare unaffected (gate returnstrue→ unchanged).Why not the alternatives
EscapeCancelsRequest: truefor Claude — simplest, but drops the vim-mode INSERT-exit entirely, even when idle.Design note
On pane-capture error,
shouldSendEscapereturnsfalse(suppress). When we can't confirm idle, not interrupting the agent is the safer default, and it's harmless for the common non-vim case where Enter alone submits. MatchesIsIdle's return-false-on-error convention.Fragility (tracked)
Detection relies on the literal
esc to interruptstatus string. This is pre-existing (shared withIsIdle/WaitForIdle), now load-bearing for nudge correctness. Documented in-code and tracked in #4240.Tests
TestShouldSendEscapeForLines— pure decision logic (busy ⇒ suppress; idle/empty ⇒ allow).TestShouldSendEscape_LivePane— real tmux pane: idle ⇒ allow,esc to interruptrendered ⇒ suppress. Capture-only, so it sidesteps thesendEnterVerifiedflakiness of the full nudge integration tests.go build/go vet/gofmtclean oninternal/tmux.Relates to #4240