Fix resize-pane for remote tmux mirror panes#7837
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:
📝 WalkthroughWalkthrough
ChangesRemote tmux pane resizing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ControlCommandCoordinator
participant TerminalController
participant RemoteTmuxWindowMirror
participant RemoteTmux
participant CmuxSocketEventMapper
CLI->>ControlCommandCoordinator: pane.resize request with cell metadata
ControlCommandCoordinator->>TerminalController: controlPaneResize(inputs)
TerminalController->>RemoteTmuxWindowMirror: requestResizePane(...)
RemoteTmuxWindowMirror->>RemoteTmux: resize-pane command
RemoteTmux-->>RemoteTmuxWindowMirror: command result
RemoteTmuxWindowMirror-->>ControlCommandCoordinator: resize response
ControlCommandCoordinator->>CmuxSocketEventMapper: map local or remote result
Possibly related issues
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 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 |
Contributor
Greptile SummaryThis PR fixes remote tmux mirror pane resizing through the control socket. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (21): Last reviewed commit: "Merge origin/main into issue-7831-mirror..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Author
|
Update after CodeRabbit inline review: refined triage supersedes the earlier rejection. Although the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/RemoteTmuxMirrorCLIFailClosedTests.swift`:
- Around line 176-182: Update
unsupportedBonsplitOnlyPaneMutationsRejectProjectedPaneIDs to retain the
disconnected-transport resize regression: invoke pane.resize using the projected
pane ID from the harness, assert it returns the unavailable failure contract,
and verify it does not return pane_not_found or mutate local Bonsplit state.
- Around line 161-172: Update the command decoding in the test to use strict
UTF-8 validation: replace String(decoding:as:) with String(bytes:encoding:),
require the result to be non-nil, then perform the existing command assertion on
the validated string.
- Around line 135-174: Add a complementary test near
paneResizeRoutesProjectedPaneToRemoteTmux that sends an absolute pane.resize
request, verifying pixel dimensions are converted to cells, the correct axis is
selected, the response payload reflects the converted resize, and the remote
tmux stream contains the expected resize-pane command using -x or -y.
In `@Sources/RemoteTmuxWindowMirror`+DividerSizing.swift:
- Around line 47-51: Replace the `first.paneIDsInOrder.first ?? 0` fallback in
the `requestResizePane` call with guarded extraction of an authoritative pane ID
from `first.paneIDsInOrder`; only issue the resize when that ID exists,
otherwise return or skip the operation without sending a tmux command.
In `@Sources/TerminalController`+RemoteTmuxControlMutations.swift:
- Around line 252-276: Prevent zero-cell absolute resizes in the absolute resize
branch of the remote mutation handler. After computing targetCells from
targetPixels and cellPixels, guard that targetCells is greater than zero and
return unavailable before calling location.mirror.requestResizePane; only
forward the request and emit remoteAbsoluteResizeRequested for valid positive
cell counts.
🪄 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: e89058f4-8e00-49e7-9dcc-4a517324266c
📒 Files selected for processing (10)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlCommandCoordinator+Pane.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlCommandCoordinator+PaneResize.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlPaneResizeResolution.swiftResources/Localizable.xcstringsSources/RemoteTmuxWindowMirror+ControlMutations.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/TerminalController+ControlPaneContext.swiftSources/TerminalController+RemoteTmuxControlMutations.swiftcmuxTests/RemoteTmuxMirrorCLIFailClosedTests.swiftcmuxTests/RemoteTmuxMirrorCLIObservabilityTests.swift
💤 Files with no reviewable changes (1)
- Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlCommandCoordinator+Pane.swift
Contributor
Author
|
CodeRabbit i18n pre-merge note: reasoned rejection. The governing repo instructions explicitly define the currently supported locales as English and Japanese. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…-7831-mirror-resize-pane
Contributor
Author
|
CodeRabbit privacy note: reasoned rejection. “Remote tmux” is the explicit user-facing feature and CLI surface ( |
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/TerminalController`+RemoteTmuxControlMutations.swift:
- Line 260: In the split-orientation handling near the remote tmux mutation
logic, replace the ternary fallback with an exhaustive axis switch that maps
"horizontal" and "vertical" explicitly and immediately returns the unavailable
result for any unrecognized axis, before split-ancestor/border checks or cell
computation; keep behavior aligned with
requestResizePane(absoluteAxis:targetCells:).
🪄 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: 5f959f37-9c67-42c5-b400-e174e5b6cd28
📒 Files selected for processing (11)
Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlPaneResizeInputs.swiftResources/Localizable.xcstringsSources/CmuxSocketEventMapper.swiftSources/RemoteTmuxNativeLayoutMetrics.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/RemoteTmuxWindowMirror+ControlMutations.swiftSources/TerminalController+RemoteTmuxControlMutations.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxMirrorCLIFailClosedTests.swiftcmuxTests/RemoteTmuxMirrorCLIObservabilityTests.swiftcmuxTests/RemoteTmuxMirrorResizeRoutingTests.swift
💤 Files with no reviewable changes (1)
- cmuxTests/RemoteTmuxMirrorCLIFailClosedTests.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
`@Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlCommandCoordinator`+PaneResize.swift:
- Around line 13-16: Update the parameter parsing in the pane resize coordinator
so explicitly supplied invalid values are rejected instead of treated as absent:
validate `target_cells`, `amount`, and `amount_cells` based on key presence,
requiring parseable positive integers where applicable, and return
`invalid_params` before resize or pixel-conversion fallback when validation
fails. Preserve defaults only when the keys are omitted.
🪄 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: fc7445b6-22c8-4477-a794-e18c9bb10caf
📒 Files selected for processing (12)
CLI/CMUXCLI+TmuxCompatSupport.swiftCLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlCommandCoordinator+PaneResize.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Pane/ControlPaneResizeInputs.swiftSources/RemoteTmuxNativeSplitTree.swiftSources/TerminalController+RemoteTmuxControlMutations.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CLITmuxCompatRemoteSplitTests.swiftcmuxTests/CLITmuxCompatResizePaneTests.swiftcmuxTests/RemoteTmuxMirrorResizeRoutingTests.swiftdaemon/remote/cmd/cmuxd-remote/tmux_compat.godaemon/remote/cmd/cmuxd-remote/tmux_corpus_behavior_test.go
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 11c9a2a. Configure here.
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
2 participants
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 #7831
Root cause
Remote-tmux panes are projected into the control-plane tree with stable pane IDs, but
pane.resizetreated every advertised pane as a local Bonsplit pane. It explicitly rejected a successful mirror lookup before entering the Bonsplit-only path, so IDs listed bycmux treeandpane.listwere reported asPane not found.Fix
ControlPaneResizeIntent, so native points, exact tmux cells, percentages, and tmux adjustment semantics cannot be silently reinterpreted later.-L/-R/-U/-D, exact-cell, or percentage semantics; positional adjustments default to one cell.pane.listnow carries calibrated cell sizes in native points so local Bonsplit fallbacks remain unit-correct on Retina displays.-yprobes inert for historical OMX HUD compatibility.swift-package-teststhrough the repository dual-Xcode runner so the SDK-15 helper build and SDK-26 package tests can both execute; the prior single-Xcode Tart runner failed before tests.unavailable/invalid-parameter errors instead of misreporting a projected pane as missing. This non-focus command preserves the user focus context.Tests
The PR retains red/green regression provenance:
pane.resizethrough the realControlCommandCoordinatorpath for a projected pane and captures the exact control-mode command.698b100f94/4c14f292eccover and implement typed intent validation, percentage forwarding, positional/default adjustments, and daemon compatibility behavior.74758e91e2/df6d07e9dbcover and implement four-pane n-ary border targeting plus calibrated point fallbacks for both Swift and Go compatibility adapters.Protocol-seam tests assert emitted
resize-panecommands; no live remote is required. An isolated tmux 3.6a four-pane check also verified that targeting B with-Rmoves B/C, while targeting C moves C/D.Local static validation (no local Xcode build/tests, per issue instructions):
git diff --check./scripts/lint-pbxproj-test-wiring.sh./scripts/check-pbxproj.shpython3 scripts/swift_file_length_budget.py --base-ref origin/mainpython3 scripts/check-package-resolved-policy.pypython3 scripts/check-workspace-package-groups.py --checkResources/Localizable.xcstringsand verifieden/jatranslations for every added keyNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches core pane layout control for both local Bonsplit and remote tmux mirrors; incorrect border targeting or unit conversion could mis-apply layouts, though behavior is heavily regression-tested.
Overview
Fixes #7831 by letting projected remote tmux pane IDs resize instead of failing as “pane not found.”
pane.resizenow resolves the layout owner first: local panes still mutate Bonsplit; mirror panes forwardresize-paneover tmux control mode and returnremote: true(surfaced aspane.resize_requested).The handler is refactored around
ControlPaneResizeIntentso tmux-compat requests carry exact cells, percentages, or directional cell counts (tmux_compat) without later reinterpretation.pane.listaddscell_*_pointsfor calibrated point fallbacks; CLI andcmuxd-remotetmux shims buildtarget_pixelsfrom those metrics and omit unsafe pixel fallbacks when geometry is missing. Remote routing picks the correct tmux target pane for n-ary/nested borders and converts outer layout points to cells (chrome-aware).Directional tmux compat defaults to 1 cell, supports attached flags like
-L7, and keeps height-only-ya deliberate no-op for OMX HUD probes. Localizedunavailable/invalid_paramserrors replace silent misrouting.Reviewed by Cursor Bugbot for commit 5b51ea2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes remote tmux mirror pane resizing by dispatching projected pane IDs to the remote tmux owner and using layout points end-to-end with chrome-aware conversion. Preserves tmux cell/percentage semantics, distinguishes applied vs requested resize events, and avoids unsafe pixel fallbacks.
Workspace.remoteTmuxControlPane/controlRemoteTmuxPaneResize; movepane.resizetoControlCommandCoordinator+PaneResize.swiftwith typedControlPaneResizeIntent.target_cells/target_percentage(or calibratedtarget_pixels); clamp deltas to ≥1 cell; honor-L/-R/-U/-D/-x; keep height‑only-ya no‑op incmuxd-remote.invalid_statewhen metrics/borders are missing.cmuxd-remote: for-x, computetarget_pixelsstrictly fromcell_*_pointsonpane.listand preserve percentages; divider sync uses the standard resize path; fix remote resize RPC assignment.pane.resizedfor local andpane.resize_requestedfor remote (remote: true); addsocket.pane.resize.invalidParametersandsocket.pane.resize.localMetricsUnavailable;pane.listnow includescell_*_points.Written for commit 5b51ea2. Summary will update on new commits.
Summary by CodeRabbit
pane.resize, including relative (direction/amount) and absolute (axis/target pixels) modes, returning richer pane and geometry details.pane.resize_requestedfor accepted remote resize requests.pane.resizedandpane.resize_requested).