Skip to content

fix: restore keyboard protocol flags after a terminal reconnect - #5088

Merged
ogulcancelik merged 3 commits into
masterfrom
fix/4985-restore-keyboard-flags-on-reconnect
Oct 11, 2026
Merged

ogulcancelik merged 3 commits into
masterfrom
fix/4985-restore-keyboard-flags-on-reconnect

Conversation

@ogulcancelik

Copy link
Copy Markdown
Collaborator

When the host terminal resets its state while Herdr keeps running, for example after VS Code "Reload Window", Herdr re-asserted mouse modes on focus-in and resize but never the kitty keyboard flags. Shift+Enter and Ctrl+Enter then arrived as plain Enter until a detach/attach.

Where Herdr already re-asserts mouse capture, it now also re-sends the keyboard modes it enabled at startup:

  • kitty flags (7, or 31 while report-all is active), only for hosts that confirmed the protocol at startup
  • modifyOtherKeys, if it was enabled
  • bracketed paste
  • on resize, focus reporting

The kitty flags use the same replace-the-stack-entry write as the report-all toggle, so repeated refreshes never grow the host's keyboard stack. Resize is the dependable trigger, because a reset host also stops sending focus events. Focus reporting is not re-enabled from a focus-in, so a host that answers the enable with a focus report can't loop. Direct attach is unchanged.

Validation

  • Exact refresh bytes for each host/mode combination: kitty host, report-all plus modifyOtherKeys, unconfirmed host, direct attach, after restore.
  • A libghostty-backed host checks that after a reset, Shift+Enter encodes as CSI 13;2u again, and that Herdr's single exit pop still restores the host.
  • With an outer kitty mode seeded under Herdr, alternating 7/31 report-all toggles with many refreshes, one exit pop restores exactly the outer mode.
  • A pty-driven client acting as a kitty host: after resize and focus-in this branch re-sends CSI < 1 u CSI > 7 u (plus 2004/1004), master re-sends none.
  • just check green, including the Windows lint stage.

VS Code itself was not tested.

refs #4985

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • ai-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9841ebde-496e-40f9-b5b3-dfb2987e365e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f9a29c11-5cc8-4920-a76b-5ddd8199d520

📥 Commits

Reviewing files that changed from the base of the PR and between 4f89611 and 88a6013.


📒 Files selected for processing (2)
  • src/client/mod.rs
  • src/client/terminal_setup.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

TerminalGuard now stores the modifyOtherKeys sequence enabled during setup and provides a method to reassert terminal keyboard modes. The client requests refreshes after qualifying Unix input, Windows focus-gained events, and resize events. Added tests cover emitted sequences, no-op conditions, host resets, and repeated report-all toggles.

Suggested reviewers: jjliebig


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 88a60

The keyboard-mode refresh is mergeable after normal checks. Behavior in VS Code remains unverified, but no specific failure is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: restoring keyboard protocol flags after a terminal reconnect.
Description check ✅ Passed The description directly explains the terminal reset issue, the keyboard protocol refresh behavior, and the validation performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Restores terminal keyboard settings after reconnect.

The PR appears safe to merge; no actionable issues were found.

What we checked:

  • Focus reports do not repeat: Only resize enables focus reporting. A focus-in event refreshes keyboard modes without enabling focus reporting again.
  • Direct attach keeps its modes: Direct attach starts with client protocols disabled. The refresh returns before writing any modes when that setting is disabled.

Summary

Herdr now re-sends its host keyboard modes on focus-in and resize. Kitty flags follow the current report-all setting, and modifyOtherKeys follows the startup setting.

  • Resize also restores focus reporting; focus-in skips it to avoid repeated focus reports.
  • Direct attach and terminal cleanup remain guarded.
  • Added tests check exact output bytes, modified Enter after a reset, and preservation of an outer keyboard mode. These tests were inspected, not run.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Focus-in or resize] --> B{Client owns modes and has not restored them?}
    B -->|No| C[Write nothing]
    B -->|Yes| D[Refresh confirmed kitty flags and startup modifyOtherKeys]
    D --> E[Enable bracketed paste]
    E --> F{Resize?}
    F -->|Yes| G[Enable focus reports]
    F -->|No| H[Leave focus reporting unchanged]
Loading

Reviews (1) · Last reviewed commit: "test: cover keyboard protocol refresh wi..." · Reviewed by Greptile

@ogulcancelik
ogulcancelik merged commit e73d21e into master Oct 11, 2026
6 checks passed
@ogulcancelik
ogulcancelik deleted the fix/4985-restore-keyboard-flags-on-reconnect branch October 11, 2026 22:06
aryrabelo added a commit to aryrabelo/bora-herdr-ada that referenced this pull request Oct 12, 2026
Upstream commits brought in:
- fix: detect codex startup update chooser (herdrdev#5174)
- fix: confirm closing a workspace's last pane in the tui (herdrdev#5173)
- fix: keep sgr pixel mouse units consistent (herdrdev#4971)
- fix: reject workspace numbers past the last workspace (herdrdev#5171)
- fix: reject oversized canonical input on macos (herdrdev#4988)
- fix: save sessions before agents die during macos restart (herdrdev#5092)
- fix: preserve hermes plugin list structure (herdrdev#4984)
- fix: restore keyboard protocol flags after a terminal reconnect (herdrdev#5088)
- fix: reduce interactive pane echo latency (herdrdev#5149)
- fix: flush session saves before replacing persisted files (herdrdev#5152)

Conflict resolutions:
- client shell close-pane confirmation: take upstream's request_pane_close,
  keep the fork's SetPaneStatus menu action and closes_group confirmation
  field; a group-root close still confirms client-side and closes the whole
  group (the fork has no server confirmation_required path, 2d839ce), so
  upstream's close_pane group test is adapted like close_tab.rs.
- pane input: keep the fork's backpressure-preserving send paths and add
  upstream's keyboard_forwarded tracking; keep send_bytes_after and
  write_user_input next to upstream's validate_input_submission.
- pane mouse: take upstream's merged encoder/position helper.
- satisfy the fork's redundant_clone deny lint in the new macos input tests.

Sets UPSTREAM_HERDR_COMMIT to 6d697e1 and bumps bora to 0.52.3.
maciejul pushed a commit to maciejul/herdr-supercharged that referenced this pull request Oct 28, 2026
…rdev#5088)

* fix: restore keyboard protocol flags after a terminal reconnect

refs herdrdev#4985

* test: cover keyboard protocol refresh with an outer kitty mode

refs herdrdev#4985
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.

2 participants