Skip to content

fix: capture held prefix command keys via kitty release events#74

Merged
kylecarbs merged 1 commit into
mainfrom
fix/prefix-key-release
Jun 17, 2026
Merged

fix: capture held prefix command keys via kitty release events#74
kylecarbs merged 1 commit into
mainfrom
fix/prefix-key-release

Conversation

@kylecarbs

@kylecarbs kylecarbs commented Jun 17, 2026

Copy link
Copy Markdown
Member

Problem

In boo ui, running a C-a prefix command while still holding the command key leaks the key's auto-repeats into the focused session. C-a c (create) prints a stray c in the new terminal. In legacy plain-byte encoding an auto-repeat is byte-for-byte identical to fresh typing, so it can't be told apart by content: the create path had no drain at all, while detach (C-a d, in both boo ui and boo attach) papered over the same race with a timed input drain (300-800ms guards, 1500ms cap).

Approach

The one signal that reliably separates a held key's tail from new typing is a key release event, which the kitty keyboard protocol provides (report event types + report all keys). This depends on terminal capability, not the focused app's mode, so boo can opt the terminal into it for the brief windows where a held command key would otherwise leak.

  • boo ui (src/ui.zig): while the prefix is engaged, force kitty report-events on the real terminal so the command key's auto-repeats (event 2) and release (event 3) arrive as explicit CSI-u events. The parser swallows them (and modifier-key chord noise) until the key is released, then the flags revert. Fixes C-a c deterministically with no delay.
  • boo attach / boo ui quit (src/client.zig): the post-detach drain re-enables report-events and ends the instant the triggering key's release arrives, instead of waiting out the timed guard. The timed guard stays as the fallback.

Terminals without the kitty protocol ignore the forced flags and fall back to the existing legacy drain, which is the best achievable without release events.

Testing

  • 10 new unit tests: the parser swallow latch (src/ui.zig) and the ReleaseScan drain detector (src/client.zig).
  • 1 new PTY integration test: arming forces \x1b[=11;1u; the command key's release reverts it.
  • Green on Linux and macOS CI: zig fmt --check, zig build test (127 tests), zig build test-integration (70 tests), and zig build test-all -Doptimize=ReleaseSafe (197 tests).
Design notes and decision log

Why release events. In legacy/modifyOtherKeys encoding a held key's auto-repeat is identical to fresh typing, so the old code could only guess with a timer. The kitty protocol's explicit press/repeat/release events are the only deterministic way to know the key is still down. So the fast path is driven by the release event, and the timer survives only as a fallback.

Terminal capability, not app mode. boo forces the flags itself for the brief prefix/teardown window, so the deterministic path works even for apps that never used kitty (e.g. a plain shell or cat). No capability handshake is needed: the forced flags are a harmless no-op on terminals that don't implement the protocol.

Why report all keys (flag 8). Unmodified command keys like c/d are delivered as raw bytes under plain disambiguate mode, so event types alone wouldn't tag their repeats. Flag 8 makes them CSI-u so the latch can act.

Graceful degradation / safety.

  • The parser only drops event-2/3 CSI-u for the dispatched key's codepoint; those events exist only under report-events, so the latch is inert on legacy terminals and self-heals when the protocol is absent.
  • The client drain matches the trigger key's release (via effective codepoint), so a C-a C-d chord's Ctrl release does not end the drain while d is still held.
  • Modifier-key noise reported under report-all is dropped during the swallow rather than forwarded.

Restoring the terminal without TCSAFLUSH (macOS). The post-detach restore drains held-key input, then hands the tty back. It used tcsetattr(TCSAFLUSH), but on Darwin the output-drain half of TCSAFLUSH blocks until the PTY master has consumed boo's writes. When the peer momentarily stops reading (exactly what the held-key detach tests do between reads, and what a stalled remote link does), the restore wedges inside tcsetattr; when it later unblocks, the input-flush half discards the input typed in the meantime, so the next reader never sees it. boo now discards straggler input non-blockingly (zero-timeout poll/read in the still-raw mode) and applies the saved mode with TCSANOW, which never waits on output. Linux's tcsetattr does not wait on PTY output, which is why this only surfaced on macOS CI.

Known limitations (documented, not fixed here).

  • On a terminal with no kitty support, legacy C-a c can still cosmetically leak a repeated c, and detach still relies on the timed guard. This is unavoidable without release events.
  • If the user presses a different key while still physically holding the command key, that one key can arrive report-all-encoded before the latch clears. Rare and cosmetic.

Out of scope / follow-up. The daemon-side keys.Parser (used by boo attach) could grow the same release-swallow for non-detach commands (C-a l redraw, C-a a literal) held over a kitty terminal; it needs the daemon to push report-events to the client and was left out to keep this change focused. Detach leaks over boo attach are already handled by the client drain.


This PR was generated by Coder Agents on behalf of @kylecarbs.

@kylecarbs kylecarbs force-pushed the fix/prefix-key-release branch from bfe4d4f to ca69cab Compare June 17, 2026 18:59
@kylecarbs kylecarbs changed the title fix: capture held prefix command keys via kitty release events fix: stop held prefix command keys leaking into boo ui sessions Jun 17, 2026
In `boo ui`, running a `C-a` prefix command while still holding the
command key leaked the key's auto-repeats into the focused session:
`C-a c` (create) printed a stray `c` in the new terminal. In legacy
plain-byte encoding an auto-repeat is byte-for-byte identical to fresh
typing, so it cannot be told apart by content.

The one deterministic signal that a key is still held is a key release
event, which the kitty keyboard protocol provides. boo forces the
report-events flags on the real terminal for the brief windows where a
held command key would otherwise leak:

- boo ui: while the C-a prefix is engaged, force report-events so the
  command key's auto-repeats (event 2) and release (event 3) arrive as
  CSI-u events. The parser swallows them, plus modifier-key chord noise,
  until the key is released, then the flags revert. Fixes C-a c with no
  delay.
- boo attach and boo ui quit: the post-detach input drain re-enables
  report-events and ends the instant the triggering key's release
  arrives instead of waiting out the timed guard, which stays as the
  fallback.

Terminals without the kitty protocol ignore the forced flags and keep
the prior timed-drain behavior; the latch self-heals when no release
events can arrive.

Restoring the terminal after the drain no longer uses TCSAFLUSH. On
Darwin its output-drain half blocks until the PTY master has consumed
the writes boo just made, so a peer that has briefly stopped reading
(a detach test between reads, a stalled remote link) wedges the restore,
and the input-flush half then discards input typed afterwards. boo now
discards straggler input non-blockingly and applies the saved mode with
TCSANOW.

Tests: parser swallow-latch and ReleaseScan unit tests, plus a PTY
integration test that arming forces report-events and the release
reverts it. The pre-existing held-key detach tests exercise the restore
path.
@kylecarbs kylecarbs force-pushed the fix/prefix-key-release branch from ca69cab to 3c5e65a Compare June 17, 2026 19:40
@kylecarbs kylecarbs changed the title fix: stop held prefix command keys leaking into boo ui sessions fix: capture held prefix command keys via kitty release events Jun 17, 2026
@kylecarbs kylecarbs merged commit 252a13a into main Jun 17, 2026
5 checks passed
@kylecarbs kylecarbs mentioned this pull request Jun 17, 2026
@kylecarbs kylecarbs deleted the fix/prefix-key-release branch June 17, 2026 20:07
BenLocal added a commit to BenLocal/boo that referenced this pull request Jun 18, 2026
Brings in upstream coder#73 (boo ui sessions created at the viewport size),
coder#74 (capture held prefix keys via kitty release events), coder#75 (scrollback
replay on attach), and the v0.5.21/v0.5.22 releases.

Conflict resolution:
- main.zig / daemon.zig: union the createSession / runDaemon / Options
  params — self's state_dir/cwd/max_scrollback (restore + config) plus
  main's rows/cols (viewport-size fix). restoreOne passes null rows/cols.
- scrollback replay (ui.zig, daemon.zig, protocol.zig, client.zig): self
  and main implemented the same feature two ways. Kept main's design (a
  separate `.ui` marker message before a SizePayload attach) and dropped
  self's redundant AttachPayload.ui flag, so client and daemon agree on
  the wire.
- window.zig: kept self's added alt-screen historyReplay test.

Verified: zig fmt, zig build, unit tests, and PTY integration tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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