Skip to content

fix(minimald): reset terminal on detach - #431

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/detach
Jun 16, 2026
Merged

fix(minimald): reset terminal on detach#431
twitchyliquid64 merged 1 commit into
mainfrom
tom/detach

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes
    • Improved terminal state cleanup when sessions disconnect or are replaced. Terminal settings (input modes, cursor visibility, colors, focus reporting) are now properly reset during teardown, preventing terminal display issues when reconnecting or switching connections.

@coderabbitai

coderabbitai Bot commented Jun 16, 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

Run ID: 5f17a60b-7efa-46ba-95a5-aa286257df12

📥 Commits

Reviewing files that changed from the base of the PR and between 768d6c4 and 243fa14.

📒 Files selected for processing (1)
  • crates/minimald/src/session_host.rs

📝 Walkthrough

Walkthrough

BindingMsg teardown variants (TeardownDueToSuperceded, TeardownDueToDetach) are changed to carry a Vec<u8> payload. A new Host::unwind_codes method generates terminal reset escape sequences from current screen state. The ctrl-w detach path and the re-attach supersede path call unwind_codes and pass the bytes into the updated message variants; the binding task writes those bytes to the SSH channel before sending the disconnect/detach text.

Changes

Terminal unwind bytes on binding teardown

Layer / File(s) Summary
BindingMsg payload contract and unwind_codes helper
crates/minimald/src/session_host.rs
TeardownDueToSuperceded and TeardownDueToDetach variants now carry Vec<u8>; Host::unwind_codes is added to build terminal reset sequences (input mode diff, alternate screen, hidden cursor, SGR reset, focus-reporting disable) from current screen state.
Teardown call sites and binding task handler
crates/minimald/src/session_host.rs
The ctrl-w detach path and the attach re-binding path call unwind_codes and pass bytes into the updated variants; the binding task's send loop writes unwind bytes to the SSH channel before emitting disconnect/detach text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit twitched its nose and said,
"Before we say goodbye, reset instead!"
🐇 Unwind the screen, the cursor, the glow,
Then send the bytes before letting go.
Clean exits only — that's the way we hop! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(minimald): reset terminal on detach' directly and specifically describes the main change - implementing terminal reset functionality when a session is detached.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) June 16, 2026 20:21
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