Preserve ssh-tmux pane identity across remote layout changes - #7838
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemote tmux mirroring now preserves eligible pane panels and stable control identities during topology reconciliation, routes pane operations through session-owned locations, adds regression coverage for layout changes and splits, and runs the focused suite through dedicated CI validation. ChangesRemote tmux identity reconciliation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
austinywang
marked this pull request as ready for review
July 10, 2026 09:57
Contributor
Greptile SummaryThis PR preserves remote tmux pane identity across layout changes. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (21): Last reviewed commit: "Coalesce remote tmux topology refreshes" | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…-7833-mirror-incremental-split
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift (1)
285-291: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
tearDownshould be called reliably even on test failure.If a test throws or records an issue before reaching
tearDown(), the pipe handles and session mirror observer won't be cleaned up, potentially leaking resources across tests. Consider callingharness.tearDown()in adeferblock at the top of each test, or conforming to a cleanup protocol if Swift Testing supports it in this codebase.♻️ Suggested pattern
+ defer { harness.tearDown() } // ... test body ... -harness.tearDown()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift` around lines 285 - 291, Ensure cleanup runs even when tests fail by registering teardown immediately at the start of each test in RemoteTmuxMirrorLayoutIdentityTests, preferably with defer or the project’s supported cleanup mechanism. Reuse the existing tearDown() cleanup for sessionMirror, workspace, panels, writer, and pipe handles, and avoid relying on reaching tearDown() through normal test completion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift`:
- Around line 239-249: Deduplicate the lookup logic shared by the windowMirror
property and windowMirror(windowID:) method. Consolidate the panel traversal
into a single helper or make the computed property delegate to
windowMirror(windowID:), applying the windowId filter only when provided while
preserving the current first-match behavior.
---
Outside diff comments:
In `@cmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift`:
- Around line 285-291: Ensure cleanup runs even when tests fail by registering
teardown immediately at the start of each test in
RemoteTmuxMirrorLayoutIdentityTests, preferably with defer or the project’s
supported cleanup mechanism. Reuse the existing tearDown() cleanup for
sessionMirror, workspace, panels, writer, and pipe handles, and avoid relying on
reaching tearDown() through normal test completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c821bebc-8b0b-4c63-afda-9756f55b87f3
📒 Files selected for processing (1)
cmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/RemoteTmuxSessionMirror.swift`:
- Around line 204-223: Update the panel mapping logic in the loop over
panelIdByWindow to prefer the pane previously associated with that panel in
previousPanelIdByPane when it still belongs to the current window’s
paneIDsInOrder; only fall back to paneIDsInOrder.first when no prior association
exists, preserving existing panel identity and scrollback during reordering or
splits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 061d2408-af7a-44f4-b5e7-fe1830feaf69
📒 Files selected for processing (22)
Sources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+LayoutPublication.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxControlPane.swiftSources/RemoteTmuxControlPaneLocation.swiftSources/RemoteTmuxControlPaneMutationOwner.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxSessionMirror+ControlTopology.swiftSources/RemoteTmuxSessionMirror+WindowReconciliation.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxWindowMirror+ControlMutations.swiftSources/RemoteTmuxWindowMirror+ControlTopology.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+ControlPaneContext.swiftSources/TerminalController+RemoteTmuxControlMutations.swiftSources/TerminalController+RemoteTmuxControlRefs.swiftSources/TerminalController+RemoteTmuxControlTopology.swiftSources/Workspace+RemoteTmuxControlTopology.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxMirrorCLIObservabilityTests.swiftcmuxTests/RemoteTmuxMirrorLayoutIdentityTests.swift
💤 Files with no reviewable changes (1)
- Sources/RemoteTmuxWindowMirror+ControlTopology.swift
# Conflicts: # .github/workflows/ci.yml
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dc526f2. Configure here.
austinywang
enabled auto-merge (squash)
July 10, 2026 15:23
# Conflicts: # cmuxTests/RemoteTmuxMirrorCLIObservabilityTests.swift
3 of 6 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7833
Root cause
The one-pane mirror path and multi-pane window renderer independently owned local pane surfaces. The first remote split crossed that representation boundary without transferring identity, so the multi-pane renderer recreated every pane and exported fresh surface and pane IDs. Bonsplit render-node IDs were also exposed as durable control-plane identities even though a fallback render-tree rebuild may replace them.
Fix
TerminalPaneland its exported pane identity when a live window first becomes multi-pane, keyed by the authoritative tmux pane ID.The reconciliation stays synchronously owned by the existing
@MainActormirror; it adds no tasks, locks, timers, or duplicate topology source of truth.Tests
The first commit adds a failing behavior regression through the real control-mode parser, pane-rect publication, topology notification, and session/window reconcile path. It covers:
The second commit implements the fix so the same behavior path passes.
Note
Medium Risk
Touches remote tmux topology, window-close race handling, and control-plane identity routing; regressions could break automation handles or pane moves, but behavior is heavily covered by new identity tests and a dedicated CI gate.
Overview
Stable automation handles for ssh-tmux mirrors no longer reset when a tab goes from one pane to splits, when panes move between windows, or when tmux publishes layout out of order.
Session-scoped control identity replaces per-window-mirror pane IDs:
RemoteTmuxSessionMirrorowns a tmux-pane→PaneIDledger, reconciles it on topology rebuild (including panes retained while a closed window’s panes might still exist elsewhere), and routes focus/input/split/resize/kill throughRemoteTmuxControlPaneLocationandRemoteTmuxControlPaneMutationOwner(session mirror in production; standalone window mirror only when no session is bound).Window mirror lifecycle adopts the existing single-pane
TerminalPanelwhen a window first becomes multi-pane (keyed by tmux pane id), creates/closes panels incrementally on layout changes, and notifies the session mirror on surface attach/detach instead of owning control cleanup locally.Control connection tracks
publishedWindowIdByPane, coalesces in-flightlist-windows, tags each snapshot withretainedPaneIDsfrom overlapping%window-closeevents, releases retention only when that snapshot succeeds, and reconnects if a retention refresh fails—so panes aren’t pruned during move/close races.%window-closenow triggers an immediaterequestWindows()while tabs can drop early.Control plane / workspace resolution goes through the session mirror when present (
remoteTmuxSessionMirror,isRemoteTmuxControlContainer); socket handlers calllocation.requestSplit/requestKilletc. rather than talking only toRemoteTmuxWindowMirror.CI: non-tolerant focused run for
RemoteTmuxMirrorLayoutIdentityTestson the app-host regression shard, plus shard script exclusions for that suite.Reviewed by Cursor Bugbot for commit bdcc92f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
CI