Skip to content

fix(input): preserve modifyOtherKeys key releases - #2303

Merged
ogulcancelik merged 2 commits into
masterfrom
akbash/2302-preserve-kitty-flag-3-releases
Aug 4, 2026
Merged

fix(input): preserve modifyOtherKeys key releases#2303
ogulcancelik merged 2 commits into
masterfrom
akbash/2302-preserve-kitty-flag-3-releases

Conversation

@akbash-bot

Copy link
Copy Markdown
Collaborator

Summary

  • request outer Kitty report-all input when the focused pane combines event-type reporting with modifyOtherKeys=2
  • preserve legacy, MOK1, and event-type-only behavior while forwarding printable press/repeat/release events in the pane's negotiated encoding
  • cover the exact Bubble Tea v2 negotiation, release routing, and MOK reset

Checks

  • cargo test host_report_all_supplies_printable_releases_for_event_type_only_panes -- --nocapture
  • cargo test host_report_all -- --nocapture
  • cargo clippy --all-targets --locked -- -D warnings
  • cargo nextest run --locked -E 'all()' --no-fail-fast ... (3186 passed; one unrelated live-handoff helper failure because Kennel's isolated CARGO_TARGET_DIR is outside the checkout path the helper accepts)
  • just windows-lint
  • maintenance Python suite (93 passed)
  • integration asset tests (19 passed)
  • plugin marketplace tests (24 passed)

refs #2302

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bb68470-f7c0-4d05-8b26-3d041114cc2e

📥 Commits

Reviewing files that changed from the base of the PR and between 1997b88 and 1e6c8b8.

📒 Files selected for processing (2)
  • src/app/input/terminal.rs
  • src/app/mod.rs

📝 Walkthrough

Walkthrough

Host keyboard report-all detection now includes event-type reporting with modify_other_keys enabled. An asynchronous regression test verifies printable press and repeat events, encoded releases, and disabling report-all requests after protocol transitions.

Changes

Keyboard report-all handling

Layer / File(s) Summary
Report-all detection and regression coverage
src/app/input/terminal.rs, src/app/mod.rs
Host report-all detection now includes event-type reporting with modify_other_keys enabled. The regression test verifies press, repeat, release, and mode-transition behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the input-handling fix for preserving modifyOtherKeys key releases.
Description check ✅ Passed The description directly explains the input-handling changes, compatibility goals, regression coverage, and validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch akbash/2302-preserve-kitty-flag-3-releases

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

@kangal-bot kangal-bot added the ai-review Trigger automated AI reviews for pull requests admitted by the PR gate label Aug 4, 2026
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

The PR expands host Kitty keyboard reporting when the focused pane combines event-type reporting with modifyOtherKeys, preserving printable key-release events in the pane’s negotiated encoding.

  • Extends the focused-pane host report-all predicate for the combined negotiation.
  • Adds coverage for printable press, repeat, and release routing and modifyOtherKeys reset behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/app/input/terminal.rs Extends host report-all detection to focused panes combining Kitty event-type reporting with active modifyOtherKeys.
src/app/mod.rs Adds regression coverage for combined protocol negotiation, printable release forwarding, and modifyOtherKeys reset behavior.

Sequence Diagram

sequenceDiagram
  participant Pane as Focused pane
  participant App as App input state
  participant Host as Host terminal
  participant PTY as Pane PTY
  Pane->>App: Negotiate event types + modifyOtherKeys
  App->>Host: Request Kitty report-all
  Host->>App: Printable press/repeat/release
  App->>PTY: Encode using pane protocol
  Pane->>App: Reset modifyOtherKeys
  App->>Host: Disable report-all
Loading

Reviews (2): Last reviewed commit: "Merge branch 'master' into akbash/2302-p..." | Re-trigger Greptile

@scrappylabsai

Copy link
Copy Markdown

Built this branch at c1b1e94 and verified it against the case from #2302 — reporting back since
the unit test covers the byte routing, and this covers a real Bubble Tea app in a real pane.

Same probe, same keys, same host terminal (Ghostty 1.3.1), only the herdr binary differs:

key presses key releases
herdr 0.8.0 stable 57 0
this branch 70 6

Releases now arriving, printable keys included:

18:13:13.040 RELEASE String()="space"
18:13:14.841 RELEASE String()="space"
18:13:16.715 RELEASE String()="p"
18:13:18.029 RELEASE String()="space"
18:13:18.987 RELEASE String()="j"
18:13:20.120 RELEASE String()="space"

The app that prompted the report — a push-to-talk dictation mode that holds a key to record and
stops on key-up — also works correctly inside a pane on this build. On stable it armed and then
never received the key-up, so the recording could not be ended.

SupportsEventTypes() reports true in both, which is why the stable behaviour was hard to tell
apart from an application bug.

Nothing needed on my end — just closing the loop with a measurement.

@ogulcancelik
ogulcancelik merged commit d57cefb into master Aug 4, 2026
8 checks passed
@ogulcancelik

Copy link
Copy Markdown
Collaborator

Built this branch at c1b1e94 and verified it against the case from #2302 — reporting back since the unit test covers the byte routing, and this covers a real Bubble Tea app in a real pane.

Same probe, same keys, same host terminal (Ghostty 1.3.1), only the herdr binary differs:

key presses key releases
herdr 0.8.0 stable 57 0
this branch 70 6
Releases now arriving, printable keys included:

18:13:13.040 RELEASE String()="space"
18:13:14.841 RELEASE String()="space"
18:13:16.715 RELEASE String()="p"
18:13:18.029 RELEASE String()="space"
18:13:18.987 RELEASE String()="j"
18:13:20.120 RELEASE String()="space"

The app that prompted the report — a push-to-talk dictation mode that holds a key to record and stops on key-up — also works correctly inside a pane on this build. On stable it armed and then never received the key-up, so the recording could not be ended.

SupportsEventTypes() reports true in both, which is why the stable behaviour was hard to tell apart from an application bug.

Nothing needed on my end — just closing the loop with a measurement.

thanks for verifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Trigger automated AI reviews for pull requests admitted by the PR gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants