fix: preserve focused viewer geometry during activation - #3746
fix: preserve focused viewer geometry during activation#3746akbash-bot wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe active shell surface path now claims tab geometry only when the activating client did not already own a previously focused tab. New integration tests cover background activation without geometry changes and focused-surface reassertion that restores geometry ownership and runtime size. Suggested reviewers: Merge Risk: ⚪ Minimal · up to Background SSH surface activation now preserves the focused viewer’s terminal size, while focused reassertion still restores sizing control. The covered behavior is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
Greptile SummaryThis PR attempts to preserve a focused viewer's terminal geometry when another client activates the same shared tab.
Confidence Score: 4/5The PR is not safe to merge because the normal presentation-sync request can immediately undo the geometry preservation. A normal endpoint activation sends two active-surface requests; after the first marks the surface active, the second takes the unchanged path and allows the background client to reclaim and resize the focused viewer's tab. Files Needing Attention: src/server/headless/surface_interest.rs, src/server/headless/tests/surface_interest.rs
|
| Filename | Overview |
|---|---|
| src/server/headless/surface_interest.rs | Adds focused-tab geometry preservation for changed activations, but unchanged activation-phase reassertions bypass it. |
| src/server/headless/tests/surface_interest.rs | Adds useful geometry regression tests, but the background-client test omits the presentation-sync reassertion that follows normal activation. |
Sequence Diagram
sequenceDiagram
participant B as Background client
participant S as Headless server
participant F as Focused viewer
B->>S: "SurfaceSet(active=true)"
Note over S: changed=true
S-->>F: Preserve focused geometry
B->>S: "Presentation-sync SurfaceSet(active=true)"
Note over S: changed=false
S->>S: claim_shell_tab_geometry(B)
S-->>F: Shared tab resized to B's geometry
Reviews (1): Last reviewed commit: "fix: preserve focused viewer geometry du..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let focused_viewer_already_owns_tab = changed | ||
| && self | ||
| .shell_tab_id_for_client(client_id) | ||
| .is_some_and(|tab_id| focused_tabs_before.contains(&tab_id)); |
There was a problem hiding this comment.
A normal activation sends another active: true request during presentation synchronization. Because the first request already marked the surface active, this second request has changed == false, calls claim_shell_tab_geometry, and resizes the shared tab to the background client's geometry. This undoes the focused viewer's geometry protection before activation completes. The new regression test checks only the first request, so it does not cover this normal second phase.
Backport herdrdev/herdr@8c5208c and herdrdev/herdr@c2d200e. Retain deferred startup initialization and normal focus/input authority transfers. Refs herdrdev#3746, herdrdev#3744.
Append eleven coherent mailboxes without changing stable BASE. Preserve the reviewed foundation except total-count subjects, retain source credit and provenance, and reproduce accepted source 53fd505 at tree 66dcebc. All 24 Windows prefixes compile; the current installer is unchanged. Refs herdrdev#3661, herdrdev#3701, herdrdev#3721, herdrdev#3651, herdrdev#3592, herdrdev#3589, herdrdev#3770, herdrdev#3779, herdrdev#3742, herdrdev#3735, herdrdev#3781, herdrdev#3778, herdrdev#3787, herdrdev#3766, herdrdev#3755, herdrdev#3754, herdrdev#3807, herdrdev#3764, herdrdev#3760, herdrdev#3746, herdrdev#3744, herdrdev#3819, herdrdev#3759
2 participants
Issue
Switching an SSH surface in a background client resizes a shared pane that another client is actively viewing. The focused client can lose the bottom or right edge of its terminal UI.
Problem
Surface activation always claimed the shared tab's size before the following host-focus update arrived. Its presentation-sync reassertion claimed the size again, so an unfocused client could replace the size selected by a focused viewer.
How did we fix it?
Herdr now keeps the focused viewer's tab size while another same-tab client activates and synchronizes its surface. Focused surface reassertions and normal focus, navigation, and input still transfer sizing control.
Verification
The regression test failed before the change with 99×35 instead of 67×17 immediately after activation. It now preserves 67×17 through activation, focus=false, and presentation synchronization. The focused reassertion test still transfers control. Surface-interest tests and CI pass on Ubuntu, macOS, Windows, and the Windows ConPTY package check.
refs #3744