fix: Prevent terminal screen wipe from Kitty keyboard protocol

[kshivang]

Summary

• Fix terminal screen being wiped after CLI tools like claude setup-token exit
• CSI < u (Kitty keyboard pop) was misinterpreted as CSI u (cursor restore), moving cursor to stale position near top of screen, then zsh's ESC[J erased all visible content
• CSI sequences with unrecognized private parameter prefixes are now silently ignored
• Harden synchronized output mode (?2026) to prevent rendering intermediate states during rapid toggle

Test plan

• Run claude setup-token in BossTerm — token output should remain visible after command completes
• Run claude access-token — same behavior, output stays on screen
• Verify spinner animations (claude CLI) render cleanly without flicker
• Verify CSI s / CSI u (cursor save/restore) still works for zsh-autosuggestions
• Verify TUI apps (vim, htop) still render correctly

🤖 Generated with Claude Code

[claude]

Code Review - PR #244

Summary

This PR fixes a critical bug where terminal output was being wiped after CLI tools exit. The root cause was that CSI < u (Kitty keyboard protocol pop) was being misinterpreted as CSI u (cursor restore), causing the cursor to move to a stale position and subsequent ESC[J sequences to clear visible content.

---

Positive Findings

1. Root Cause Analysis ✅

Excellent diagnosis - the issue was correctly identified as a parsing problem where the < character in CSI < u was being treated as an unhandled char and pushed back, resulting in < being written to screen + bare CSI u being processed as cursor restore.

2. Elegant Solution ✅

The fix in BossEmulator1.kt:159-167 is clean and surgical with minimal code changes and maximum impact.

3. Race Condition Hardening ✅

The synchronized update mode improvements are well-thought-out with triple-check pattern to prevent TOCTOU issues.

---

Issues & Concerns

1. Missing Test Coverage ⚠️ HIGH PRIORITY

No automated tests were added for this critical bug fix. The codebase has existing test infrastructure but no tests cover CSI sequence parsing with private parameters or synchronized update mode race conditions. This bug could regress without detection.

Recommendation: Add unit tests for CSI parsing logic with unhandled characters.

2. Incomplete Private Parameter Handling ⚠️ MEDIUM PRIORITY

The code explicitly handles !, ?, >, = as position-0 private parameters, but < falls through to addUnhandled(). This means you're silently ignoring ALL sequences with <, not just keyboard-related ones. Either add explicit handling for < similar to other private parameters, or document clearly that ignoring all <-prefixed sequences is intentional.

3. Debug Logging Consistency ⚠️ LOW PRIORITY

The new ignore path logs at DEBUG level, but the normal path logs unhandled sequences at WARN level. Consider logging ignored sequences at INFO or WARN level with a rate limiter.

4. Synchronization Performance Impact 🔍 NEEDS INVESTIGATION

You've added three synchronized blocks in hot rendering paths. The cumulative impact on high-frequency terminal output is unclear. Recommend benchmarking rendering performance with and without these changes.

---

Security Considerations

No concerns identified - no unsafe input parsing, command injection vectors, or timing attack vulnerabilities.

---

Code Quality

Strengths: Clear comments, follows existing style, proper Kotlin idioms, good variable naming.

Minor nitpicks: Line 330/345 ?: continue is clever but cryptic - consider extracting to a helper method.

---

Recommendations Summary

• HIGH: Add unit tests for CSI parsing logic
• HIGH: Document whether ignoring ALL < sequences is intentional
• MEDIUM: Consider explicit < parameter handling vs blanket ignore
• MEDIUM: Benchmark rendering performance impact
• LOW: Improve logging for ignored sequences

---

Verdict

Recommendation: Approve with Conditions

This PR correctly identifies and fixes a real bug with a minimal solution. The synchronized update hardening is valuable. However, lack of automated tests and blanket ignoring of <-prefixed sequences should be addressed before or after merging.

Blocking Issues: None (but test coverage should be prioritized in follow-up)

Great work on the debugging and fix! 🎯