Skip to content

Filter terminal reply traffic around tmux attach and detach#588

Open
johnrichardrinehart wants to merge 1 commit intoasheshgoplani:mainfrom
johnrichardrinehart:fix/terminal-reply-filter
Open

Filter terminal reply traffic around tmux attach and detach#588
johnrichardrinehart wants to merge 1 commit intoasheshgoplani:mainfrom
johnrichardrinehart:fix/terminal-reply-filter

Conversation

@johnrichardrinehart
Copy link
Copy Markdown
Contributor

The attached-session path can leak terminal reply bytes back into Agent Deck's
Bubble Tea UI during rapid attach/detach cycles. In practice this showed up as
literal fragments such as (0.44.0)/d3d3/f5f5/d3d3/f5f5(0.44.0)... appearing
in the TUI after repeating:

  • attach to a tmux-backed session
  • detach with Ctrl+Q
  • reattach and press Enter a few times

Relevant environment from the repro:

  • Agent Deck running inside tmux
  • Bubble Tea UI on the host side
  • terminal-emulator replies arriving on stdin during redraw / teardown
  • the visible reply fragments included terminal version and color-report text

The underlying problem is that terminal protocol replies are not user input, but
we were still letting them cross the attach/detach boundary. The fix here keeps
that handling bounded to those lifecycle edges and treats the problem from first
principles:

  • add a small stateful terminal-reply filter that understands protocol classes
    instead of matching terminal-specific payload strings
  • discard escape-string replies generically (OSC, DCS, APC, PM, SOS)
  • discard non-keyboard CSI replies during the short quarantine window
  • preserve normal keyboard input, including CSI/SS3 key sequences such as
    arrows, Home/End, Backtab, and CSI-u key events
  • reuse the same filter in both the tmux attach path and the keyboard
    compatibility reader so split replies are handled consistently across reads

This intentionally avoids keying behavior to a specific terminal emulator,
terminal version, or tmux version. The parser is based on control-sequence
structure, not on reply payload contents.

Constraint: normal keyboard input must survive immediately after attach/detach,
including CSI/SS3-based keys

Rejected alternatives:

  • match terminal-specific payload strings
    • too brittle against emulator or multiplexer version changes
  • drop all CSI traffic during the quarantine window
    • would break legitimate keyboard input
  • keep extending PTY drain timeouts alone
    • the failing bytes were arriving on stdin as protocol replies, not just on
      PTY output

Tested:

  • nix shell nixpkgs#go nixpkgs#gcc -c go test ./internal/tmux ./internal/ui ./internal/termreply -run 'TestCleanupAttach_|TestCSIuReader(DropsTerminalRepliesDuringQuarantine|DropsSplitTerminalRepliesDuringQuarantine|PreservesNormalInputDuringQuarantine)|TestFilter'

Not tested:

  • a fresh packaged build exercised manually through repeated Ctrl+Q / Enter
    cycles after this exact commit

The attached-session path can leak terminal reply bytes back into Agent Deck's
Bubble Tea UI during rapid attach/detach cycles. In practice this showed up as
literal fragments such as `(0.44.0)/d3d3/f5f5/d3d3/f5f5(0.44.0)...` appearing
in the TUI after repeating:

- attach to a tmux-backed session
- detach with `Ctrl+Q`
- reattach and press `Enter` a few times

Relevant environment from the repro:

- Agent Deck running inside tmux
- Bubble Tea UI on the host side
- terminal-emulator replies arriving on stdin during redraw / teardown
- the visible reply fragments included terminal version and color-report text

The underlying problem is that terminal protocol replies are not user input, but
we were still letting them cross the attach/detach boundary. The fix here keeps
that handling bounded to those lifecycle edges and treats the problem from first
principles:

- add a small stateful terminal-reply filter that understands protocol classes
  instead of matching terminal-specific payload strings
- discard escape-string replies generically (OSC, DCS, APC, PM, SOS)
- discard non-keyboard CSI replies during the short quarantine window
- preserve normal keyboard input, including CSI/SS3 key sequences such as
  arrows, Home/End, Backtab, and CSI-u key events
- reuse the same filter in both the tmux attach path and the keyboard
  compatibility reader so split replies are handled consistently across reads

This intentionally avoids keying behavior to a specific terminal emulator,
terminal version, or tmux version. The parser is based on control-sequence
structure, not on reply payload contents.

Constraint: normal keyboard input must survive immediately after attach/detach,
including CSI/SS3-based keys

Rejected alternatives:

- match terminal-specific payload strings
  - too brittle against emulator or multiplexer version changes
- drop all CSI traffic during the quarantine window
  - would break legitimate keyboard input
- keep extending PTY drain timeouts alone
  - the failing bytes were arriving on stdin as protocol replies, not just on
    PTY output

Tested:

- `nix shell nixpkgs#go nixpkgs#gcc -c go test ./internal/tmux ./internal/ui ./internal/termreply -run 'TestCleanupAttach_|TestCSIuReader(DropsTerminalRepliesDuringQuarantine|DropsSplitTerminalRepliesDuringQuarantine|PreservesNormalInputDuringQuarantine)|TestFilter'`

Not tested:

- a fresh packaged build exercised manually through repeated `Ctrl+Q` / `Enter`
  cycles after this exact commit
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