render tmux windows as live multi-pane canvases - #49
Conversation
Deploying remux with
|
| Latest commit: |
d4fb711
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d580899f.remux-agx.pages.dev |
| Branch Preview URL: | https://feature-pane-composite-canva.remux-agx.pages.dev |
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR changes Remux from single-pane terminal presentation to topology-aware composite rendering. It adds pane-map geometry, zoom-aware tmux commands, renderer recovery snapshots, cursor-centered previews, measured viewport updates, updated selection sheets, and corresponding unit and UI test coverage. ChangesComposite terminal presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GhosttyCompositeViewportView
participant GhosttySurfaceScreen
participant TmuxTerminalScreenAdapter
participant TmuxSessionController
participant TmuxSessionLink
User->>GhosttyCompositeViewportView: Tap visible pane
GhosttyCompositeViewportView->>GhosttySurfaceScreen: Send surface ID and keyboard preference
GhosttySurfaceScreen->>TmuxTerminalScreenAdapter: Focus pane
TmuxTerminalScreenAdapter->>TmuxSessionController: Submit pane selection or zoom request
TmuxSessionController->>TmuxSessionLink: Send tmux commands
TmuxSessionLink-->>TmuxSessionController: Deliver output and topology updates
TmuxTerminalScreenAdapter-->>GhosttyCompositeViewportView: Update pane projection
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Deploying getremux with
|
| Latest commit: |
d4fb711
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f10b4bfc.getremux.pages.dev |
| Branch Preview URL: | https://feature-pane-composite-canva.getremux.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a42fbadcc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| viewport: selectedTopLevelID == topLevelID | ||
| ? terminalViewportPresentationProjection | ||
| : nil |
There was a problem hiding this comment.
If tmux changes the active window externally while this pane sheet is open, the sheet's original topLevelID still exists, so paneSelectionSheetTopologyProjection does not dismiss it, but this conditional now passes nil as its viewport. The projector consequently returns no pane frames or window grid, and the visible sheet becomes a blank fallback even though all of that window's panes still exist. Derive the requested window's geometry from latestTopology, or dismiss/switch the sheet when its top level stops being selected.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
RemuxApp/Sources/Tmux/TmuxPaneSurface.swift (1)
472-478: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRenderer recovery has no terminal state and no retry, so a pane can stay permanently non-interactive.
rendererDidFail()marks the pane unavailable and setsrendererFailureReported = truebefore recovery begins. Only a successful replacement clears that flag. Every path that ends recovery without success therefore parks the pane behind the recovery overlay with input disabled and no way back.
RemuxApp/Sources/Tmux/TmuxPaneSurface.swift#L472-L478: callfinishRendererReplacementon every exit of the replacement flow, including the generation-mismatch return at Line 543-546, and clearrendererFailureReported(or expose a retry entry point) when the replacement fails so a later attempt is possible.RemuxApp/Sources/Tmux/TmuxTerminalSession.swift#L260-L266: when the guard rejects the failure becausepresentationMetrics(for:in:)returns nil, record the pane as needing a replacement, then retry it fromhandleTopologyandupdateViewportMeasurementonce topology and viewport metrics resolve.🤖 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 `@RemuxApp/Sources/Tmux/TmuxPaneSurface.swift` around lines 472 - 478, Ensure the renderer replacement flow always calls finishRendererReplacement on every exit, including generation-mismatch handling, and clears rendererFailureReported or otherwise exposes retry when replacement fails so recovery can be attempted again. In RemuxApp/Sources/Tmux/TmuxPaneSurface.swift lines 472-478, update the replacement failure path; also update the generation-mismatch path in that file around lines 543-546. In RemuxApp/Sources/Tmux/TmuxTerminalSession.swift lines 260-266, record panes whose failure handling is skipped because presentationMetrics(for:in:) is nil, then retry replacement from handleTopology and updateViewportMeasurement once metrics become available.
🧹 Nitpick comments (14)
RemuxApp/Sources/Tmux/TmuxScreenModel.swift (1)
313-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a separate re-measure entry point.
applyTerminalSettingsnow callsprepareInitialViewport. That method reports.disconnected(Self.initialViewportFailureReason)and setsstartupFailurewhen measurement throws. A live, attached session then surfaces a startup-style failure after a theme change. The name also no longer matches the behavior, because the path runs after the initial connect.Consider extracting the shared measurement into a
remeasureViewport(size:scale:)helper and reporting a settings-application failure on this path instead of the initial-viewport reason.🤖 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 `@RemuxApp/Sources/Tmux/TmuxScreenModel.swift` around lines 313 - 316, Separate the post-connection measurement path in applyTerminalSettings from prepareInitialViewport by extracting shared measurement into a remeasureViewport(size:scale:) helper. Ensure remeasurement reports a settings-application failure rather than .disconnected(Self.initialViewportFailureReason) or setting startupFailure, while preserving the initial-connect behavior of prepareInitialViewport.RemuxApp/Sources/Tmux/TmuxSessionController.swift (1)
1106-1123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the zoom-aware pane-selection submission.
Lines 1106-1123 and lines 1141-1158 build the same command and choose between
submitPanePresentationCommandOnWriterandsubmitCommandOnWriterwith the same condition. Only theRequestvalue differs. One helper removes the duplicated policy and keeps both call sites in step when the zoom rules change.♻️ Proposed helper
private func submitPaneSelection( paneID: TmuxPaneID, zoomed: Bool, request: Request, drainOutbound: Bool ) { let command = zoomed ? "select-pane -Z -t %\(paneID.rawValue)" : "select-pane -t %\(paneID.rawValue)" if zoomed { submitPanePresentationCommandOnWriter( command: command, request: request, paneID: paneID, drainOutbound: drainOutbound ) } else { submitCommandOnWriter( command: command, request: request, drainOutbound: drainOutbound ) } }🤖 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 `@RemuxApp/Sources/Tmux/TmuxSessionController.swift` around lines 1106 - 1123, Extract the duplicated pane-selection logic that appears in two locations into a new private helper method named submitPaneSelection. The helper should accept paneID, zoomed, request, and drainOutbound parameters, build the zoom-aware select-pane command once, and conditionally route to either submitPanePresentationCommandOnWriter or submitCommandOnWriter based on the zoomed flag. Replace both occurrences of this pattern (lines 1106-1123 and lines 1141-1158) with calls to this helper, passing the appropriate Request value to each call site. This ensures the zoom-selection logic and command structure are defined once and both call sites stay synchronized.RemuxApp/Sources/Ghostty/GhosttyKitControlSurface.swift (1)
351-357: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA failed cursor query invalidates the whole control surface.
report(result)callsfail(.native(result))for any non-OK result.failsetsinvalidated = trueand callsonFailure, which reachesTmuxPaneSurface.rendererDidFail()and triggers a full renderer replacement.cursorGeometry()is only used to pick a preview crop anchor inTmuxPaneSurface.capturePickerPreview, so a transientGHOSTTY_TERMINAL_SURFACE_RESULT_FAILEDfrom a cosmetic query would tear down and rebuild a working renderer.
selectionSnapshot()handles the same native result class throughselectionOutcome, which retries withghostty_terminal_surface_terminal_changedinstead of failing. Consider treating a cursor-geometry failure as "no anchor" and returningnilwithout escalating.♻️ Suggested handling
func cursorGeometry() -> CGRect? { guard !invalidated else { return nil } var geometry = ghostty_terminal_surface_cell_geometry_s() let result = ghostty_terminal_surface_cursor_geometry(handle, &geometry) - guard report(result) else { return nil } + guard result == GHOSTTY_TERMINAL_SURFACE_RESULT_OK else { return nil } return ghosttyTerminalCellFrame(geometry, scaleFactor: scaleFactor) }Confirm which native results
ghostty_terminal_surface_cursor_geometrycan return before choosing the policy.🤖 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 `@RemuxApp/Sources/Ghostty/GhosttyKitControlSurface.swift` around lines 351 - 357, Update cursorGeometry() so failures from ghostty_terminal_surface_cursor_geometry are handled as a missing preview anchor rather than passed to report(result), which invalidates the control surface. Confirm the native result values this query can return, and return nil for the transient/failure result without invoking fail or onFailure; preserve the existing invalidated guard and successful geometry conversion.RemuxApp/Sources/Ghostty/TerminalSelectionSheetStyle.swift (1)
199-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
clipShapebeforeglassEffect.
glassEffect(..., in: shape)already clips toCapsule(), so the prior.clipShape(shape)is unnecessary. Keep it only if the developer specifically wants to avoid downstream overflow artifacts outside the effect path.🤖 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 `@RemuxApp/Sources/Ghostty/TerminalSelectionSheetStyle.swift` around lines 199 - 229, Remove the redundant .clipShape(shape) call from the iOS 26 branch of terminalSelectionSheetControlGroupSurface, leaving glassEffect(..., in: shape) responsible for the capsule clipping. Keep the existing overlay and fallback branch unchanged.RemuxApp/Sources/Tmux/TmuxTerminalScreenAdapter.swift (2)
73-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not capture
sessionstrongly inside the topology subscription.The adapter holds
sessionasweakon purpose. This closure captures theactivate(session:)parameter strongly, andsubscriptionsretains the closure, so the adapter keeps the session alive untilinvalidate()runs. Read the stored weak reference instead so the lifetime contract stays in one place.♻️ Proposed fix
self.rebuildTopologySnapshot() self.reconcileManagedSurfaces( - sessionSurfaces: session.surfacesByPaneID, + sessionSurfaces: self.session?.surfacesByPaneID ?? [:], topology: topology )🤖 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 `@RemuxApp/Sources/Tmux/TmuxTerminalScreenAdapter.swift` around lines 73 - 93, Update the topology subscription in activate(session:) to avoid capturing its session parameter strongly; use the adapter’s stored weak session reference when accessing surfacesByPaneID, while preserving the existing topology reconciliation behavior.
110-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBind
selfonce in the failure sink.The closure mixes
self?.calls with an innerlet selfbinding for the.closePanebranch. Bindselfat the top of the closure and use direct member access for every branch. This keeps the four failure branches symmetric.🤖 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 `@RemuxApp/Sources/Tmux/TmuxTerminalScreenAdapter.swift` around lines 110 - 129, Update the lastFailedRequest sink closure to bind self once at its start with a guard, then use direct member access throughout the selectPane, zoomPane, closePane, and presentCommandFailure branches. Remove the inner let self binding while preserving the existing request handling behavior.RemuxApp/Sources/Ghostty/PanePreviewLayout.swift (1)
135-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompute the scaled side once.
paneMapPhysicalPixelBudgetevaluates(side * safeScale).rounded(.up)twice for a square budget. Bind it to one constant to make the square intent explicit.♻️ Proposed refactor
let safeScale = max(scale, 1) let side = min(availableWidth, maximumHeight) - return ( - clampUInt32((side * safeScale).rounded(.up)), - clampUInt32((side * safeScale).rounded(.up)) - ) + let sidePx = clampUInt32((side * safeScale).rounded(.up)) + return (sidePx, sidePx)🤖 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 `@RemuxApp/Sources/Ghostty/PanePreviewLayout.swift` around lines 135 - 146, In paneMapPhysicalPixelBudget, compute the rounded scaled side once in a local constant, then pass that value to clampUInt32 for both the width and height tuple fields while preserving the existing square budget behavior.RemuxApp/Sources/Ghostty/GhosttySingleViewportView.swift (1)
5-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the file to match its primary type.
This file now defines
GhosttyCompositeViewportViewandGhosttyCompositeViewportContainerView, but the filename is stillGhosttySingleViewportView.swift. Rename it toGhosttyCompositeViewportView.swiftso the file matches the type it declares.🤖 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 `@RemuxApp/Sources/Ghostty/GhosttySingleViewportView.swift` around lines 5 - 7, Rename GhosttySingleViewportView.swift to GhosttyCompositeViewportView.swift so the filename matches the primary GhosttyCompositeViewportView type declared in the file; leave the view implementations unchanged.RemuxApp/Sources/Ghostty/GhosttySurfaceScreen.swift (1)
2072-2078: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the pane-map fallback height with the sheet.
This detent uses
160whenwindowGridor the pane-map metrics are unavailable.GhosttyPaneSelectionSheet.paneMapuses a separateminHeight: 160literal for the same fallback. If one literal changes, the detent no longer matches the rendered content. Expose one constant, for example onPanePreviewLayout, and read it in both places.🤖 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 `@RemuxApp/Sources/Ghostty/GhosttySurfaceScreen.swift` around lines 2072 - 2078, Define a shared pane-map fallback height constant on PanePreviewLayout, then replace the 160 fallback in the .panes detent calculation and the minHeight: 160 value in GhosttyPaneSelectionSheet.paneMap with that constant so both paths remain synchronized.RemuxAppTests/TmuxTerminalScreenAdapterTests.swift (2)
389-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the byte accounting after replacement.
TmuxPanePreviewImageCache.storesubtracts the replaced entry'sbyteCostbefore adding the new one. This test checks the image identity and the entry count, but nottotalByteCost. A regression that skips the subtraction would keep both assertions green and then over-count the cache, causing premature eviction.Add the byte-cost assertion.
♻️ Proposed fix
XCTAssertTrue(cache.preview(for: 1)?.image === replacement) XCTAssertEqual(cache.entries.count, 1) + XCTAssertEqual( + cache.totalByteCost, + replacement.bytesPerRow * replacement.height + )🤖 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 `@RemuxAppTests/TmuxTerminalScreenAdapterTests.swift` around lines 389 - 399, Extend testPanePreviewCacheReplacesThePreviousImageForAPane to assert that cache.totalByteCost equals the replacement preview’s byte cost after storing the second image, confirming the replaced entry’s cost was removed before the new cost was added.
190-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the pane indexes before subscripting.
This test subscripts
viewport.panes[0]andviewport.panes[1]without asserting the count first. If the projection returns fewer panes, the subscript traps and aborts the test process instead of reporting a failure. The sibling test at Line 146 asserts the count before mapping.Assert the count first, and use
XCTAssertNilfor the hidden pane.♻️ Proposed fix
let viewport = adapter.terminalScreenPresentationProjection.viewport XCTAssertTrue(viewport.isServerZoomed) - XCTAssertEqual(viewport.panes[0].visibleFrame, nil) + XCTAssertEqual(viewport.panes.count, 2) + guard viewport.panes.count == 2 else { return } + XCTAssertNil(viewport.panes[0].visibleFrame) XCTAssertEqual( viewport.panes[1].visibleFrame, .init(x: 0, y: 0, columns: 80, rows: 24) )🤖 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 `@RemuxAppTests/TmuxTerminalScreenAdapterTests.swift` around lines 190 - 200, Update the test around terminalScreenPresentationProjection to assert viewport.panes contains the expected number of panes before subscripting it, matching the sibling test’s guard pattern. Replace the equality assertion for viewport.panes[0].visibleFrame with XCTAssertNil while preserving the existing pane frame expectations.RemuxAppTests/GhosttyTerminalPresentationProjectorTests.swift (1)
481-494: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe viewport assertion now restates the test input.
The test passes
focusedSurfaceID: surfaceInstanceIDintoviewportProjection, then assertsprojection.viewport.focusedSurfaceID == surfaceInstanceIDand!= paneID. Both assertions hold for any pass-through implementation, so they no longer prove that the projector separates surface instance identity from pane identity. Only theselectedActiveLeafIDassertion still exercises derivation.Make the input distinguish the two identities. For example, include the pane surface ID in
paneswhilefocusedSurfaceIDstays the surface instance, then assert which value each output field takes.🤖 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 `@RemuxAppTests/GhosttyTerminalPresentationProjectorTests.swift` around lines 481 - 494, Update the test’s viewportProjection setup to provide distinct pane and surface-instance identities, such as including paneID in panes while retaining surfaceInstanceID as focusedSurfaceID. Revise the assertions around the projector result to verify viewport.focusedSurfaceID preserves the surface instance and interaction.selectedActiveLeafID derives the expected pane identity, rather than asserting only pass-through values.RemuxAppUITests/RemuxAppUITests.swift (1)
1170-1176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert which pane was removed, not just the count.
The previous code waited for a specific tile identifier to disappear. The new assertion only checks that the tile count drops to 1, so removing the wrong pane still passes. This test records no pane-count or pane-capture expectation afterwards, so nothing else catches a wrong-pane removal. The same pattern appears at Lines 1599-1604 in
testLiveSSHBackgroundForegroundRetainsTerminalWhenConfigured.Capture the identifier before removal and assert that exact tile disappears, in addition to the count.
♻️ Proposed fix
openPanesSheet() XCTAssertTrue(waitForPanePickerTileCount(2, timeout: 10)) - removePanePickerItem(panePickerTiles()[1]) + let removedTile = panePickerTiles()[1] + let removedIdentifier = removedTile.identifier + removePanePickerItem(removedTile) + XCTAssertTrue( + waitForElementToDisappear(app.buttons[removedIdentifier], timeout: 10), + "The removed pane tile should disappear from the picker." + ) XCTAssertTrue( waitForPanePickerTileCount(1, timeout: 10), "The removed pane should disappear from the picker." )🤖 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 `@RemuxAppUITests/RemuxAppUITests.swift` around lines 1170 - 1176, Update the pane-removal test around openPanesSheet and removePanePickerItem to capture the selected pane’s identifier before removal, then assert that exact identifier disappears while retaining the existing tile-count assertion. Apply the same change in testLiveSSHBackgroundForegroundRetainsTerminalWhenConfigured, using the existing pane-picker tile identifier and disappearance-wait helpers.RemuxAppTests/GhosttyPanePreviewSessionTests.swift (1)
231-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
.paneMappreview sizing.
GhosttyPanePreviewSession.pixelBudget()branches onpreviewSizing, andpaneMapForCurrentScreenuses.paneMap(...), butGhosttyPanePreviewSessionTests.swiftonly exercises.windowGrid(...). Add one test that mirrorstestWindowGridUsesWindowPreviewPixelBudgetand asserts.paneMap(...)requestsPanePreviewLayout.paneMapPhysicalPixelBudgetdimensions.🤖 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 `@RemuxAppTests/GhosttyPanePreviewSessionTests.swift` around lines 231 - 257, Add a test alongside testWindowGridUsesWindowPreviewPixelBudget that constructs GhosttyPanePreviewSession with .paneMap sizing, starts refreshing, waits for one capture, and asserts the request budget matches PanePreviewLayout.paneMapPhysicalPixelBudget dimensions for the same width and scale. Resolve the harness request as the existing test does.
🤖 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 `@RemuxApp/Sources/Ghostty/GhosttySurfaceSelectionSheet.swift`:
- Around line 311-347: The pane tile accessibility identifier is attached to the
outer ZStack instead of the tappable control. Move
terminal.pane.tile.\(pane.id.uuidString) from the ZStack to the pane Button,
matching the window tile’s identifier placement and preserving the existing
identifier value.
In `@RemuxApp/Sources/Ghostty/GhosttyTerminalPresentationProjector.swift`:
- Around line 730-732: Update the viewportPanesByID construction in
paneSelectionSheetRenderProjection to use the non-trapping Dictionary
initializer with a duplicate-key resolution closure, preserving one pane for
each ID instead of crashing when viewport.panes contains duplicates.
In `@RemuxApp/Sources/Tmux/TmuxTerminalSession.swift`:
- Around line 256-258: The handleActivePaneChanged method must reconcile
presentation activity when the native active-pane event occurs, rather than only
refreshing the affected surface’s interaction state. Update
handleActivePaneChanged to invoke the existing reconcilePresentationActivity
flow (using the current topology/state inputs) so display and keyboard focus are
reassigned consistently even without a topology callback.
In `@RemuxAppTests/TmuxSessionLinkWriteFailureTests.swift`:
- Around line 78-81: Update the outbound assertion in
TmuxSessionLinkWriteFailureTests to compare the received byte data directly
against the expected “new-window\n” UTF-8 bytes, removing the
String(decoding:as:) conversion while preserving the current contains behavior.
In `@RemuxAppUITests/RemuxAppUITests.swift`:
- Around line 3709-3720: The removePanePickerItem helper currently stops after
opening the pane removal confirmation dialog. After tapPickerButton invokes
terminal.pane.remove.<pane id> or its fallback label, add a tap for
terminal.pane.remove.confirm to complete the confirmed removal.
---
Outside diff comments:
In `@RemuxApp/Sources/Tmux/TmuxPaneSurface.swift`:
- Around line 472-478: Ensure the renderer replacement flow always calls
finishRendererReplacement on every exit, including generation-mismatch handling,
and clears rendererFailureReported or otherwise exposes retry when replacement
fails so recovery can be attempted again. In
RemuxApp/Sources/Tmux/TmuxPaneSurface.swift lines 472-478, update the
replacement failure path; also update the generation-mismatch path in that file
around lines 543-546. In RemuxApp/Sources/Tmux/TmuxTerminalSession.swift lines
260-266, record panes whose failure handling is skipped because
presentationMetrics(for:in:) is nil, then retry replacement from handleTopology
and updateViewportMeasurement once metrics become available.
---
Nitpick comments:
In `@RemuxApp/Sources/Ghostty/GhosttyKitControlSurface.swift`:
- Around line 351-357: Update cursorGeometry() so failures from
ghostty_terminal_surface_cursor_geometry are handled as a missing preview anchor
rather than passed to report(result), which invalidates the control surface.
Confirm the native result values this query can return, and return nil for the
transient/failure result without invoking fail or onFailure; preserve the
existing invalidated guard and successful geometry conversion.
In `@RemuxApp/Sources/Ghostty/GhosttySingleViewportView.swift`:
- Around line 5-7: Rename GhosttySingleViewportView.swift to
GhosttyCompositeViewportView.swift so the filename matches the primary
GhosttyCompositeViewportView type declared in the file; leave the view
implementations unchanged.
In `@RemuxApp/Sources/Ghostty/GhosttySurfaceScreen.swift`:
- Around line 2072-2078: Define a shared pane-map fallback height constant on
PanePreviewLayout, then replace the 160 fallback in the .panes detent
calculation and the minHeight: 160 value in GhosttyPaneSelectionSheet.paneMap
with that constant so both paths remain synchronized.
In `@RemuxApp/Sources/Ghostty/PanePreviewLayout.swift`:
- Around line 135-146: In paneMapPhysicalPixelBudget, compute the rounded scaled
side once in a local constant, then pass that value to clampUInt32 for both the
width and height tuple fields while preserving the existing square budget
behavior.
In `@RemuxApp/Sources/Ghostty/TerminalSelectionSheetStyle.swift`:
- Around line 199-229: Remove the redundant .clipShape(shape) call from the iOS
26 branch of terminalSelectionSheetControlGroupSurface, leaving glassEffect(...,
in: shape) responsible for the capsule clipping. Keep the existing overlay and
fallback branch unchanged.
In `@RemuxApp/Sources/Tmux/TmuxScreenModel.swift`:
- Around line 313-316: Separate the post-connection measurement path in
applyTerminalSettings from prepareInitialViewport by extracting shared
measurement into a remeasureViewport(size:scale:) helper. Ensure remeasurement
reports a settings-application failure rather than
.disconnected(Self.initialViewportFailureReason) or setting startupFailure,
while preserving the initial-connect behavior of prepareInitialViewport.
In `@RemuxApp/Sources/Tmux/TmuxSessionController.swift`:
- Around line 1106-1123: Extract the duplicated pane-selection logic that
appears in two locations into a new private helper method named
submitPaneSelection. The helper should accept paneID, zoomed, request, and
drainOutbound parameters, build the zoom-aware select-pane command once, and
conditionally route to either submitPanePresentationCommandOnWriter or
submitCommandOnWriter based on the zoomed flag. Replace both occurrences of this
pattern (lines 1106-1123 and lines 1141-1158) with calls to this helper, passing
the appropriate Request value to each call site. This ensures the zoom-selection
logic and command structure are defined once and both call sites stay
synchronized.
In `@RemuxApp/Sources/Tmux/TmuxTerminalScreenAdapter.swift`:
- Around line 73-93: Update the topology subscription in activate(session:) to
avoid capturing its session parameter strongly; use the adapter’s stored weak
session reference when accessing surfacesByPaneID, while preserving the existing
topology reconciliation behavior.
- Around line 110-129: Update the lastFailedRequest sink closure to bind self
once at its start with a guard, then use direct member access throughout the
selectPane, zoomPane, closePane, and presentCommandFailure branches. Remove the
inner let self binding while preserving the existing request handling behavior.
In `@RemuxAppTests/GhosttyPanePreviewSessionTests.swift`:
- Around line 231-257: Add a test alongside
testWindowGridUsesWindowPreviewPixelBudget that constructs
GhosttyPanePreviewSession with .paneMap sizing, starts refreshing, waits for one
capture, and asserts the request budget matches
PanePreviewLayout.paneMapPhysicalPixelBudget dimensions for the same width and
scale. Resolve the harness request as the existing test does.
In `@RemuxAppTests/GhosttyTerminalPresentationProjectorTests.swift`:
- Around line 481-494: Update the test’s viewportProjection setup to provide
distinct pane and surface-instance identities, such as including paneID in panes
while retaining surfaceInstanceID as focusedSurfaceID. Revise the assertions
around the projector result to verify viewport.focusedSurfaceID preserves the
surface instance and interaction.selectedActiveLeafID derives the expected pane
identity, rather than asserting only pass-through values.
In `@RemuxAppTests/TmuxTerminalScreenAdapterTests.swift`:
- Around line 389-399: Extend
testPanePreviewCacheReplacesThePreviousImageForAPane to assert that
cache.totalByteCost equals the replacement preview’s byte cost after storing the
second image, confirming the replaced entry’s cost was removed before the new
cost was added.
- Around line 190-200: Update the test around
terminalScreenPresentationProjection to assert viewport.panes contains the
expected number of panes before subscripting it, matching the sibling test’s
guard pattern. Replace the equality assertion for viewport.panes[0].visibleFrame
with XCTAssertNil while preserving the existing pane frame expectations.
In `@RemuxAppUITests/RemuxAppUITests.swift`:
- Around line 1170-1176: Update the pane-removal test around openPanesSheet and
removePanePickerItem to capture the selected pane’s identifier before removal,
then assert that exact identifier disappears while retaining the existing
tile-count assertion. Apply the same change in
testLiveSSHBackgroundForegroundRetainsTerminalWhenConfigured, using the existing
pane-picker tile identifier and disappearance-wait helpers.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cc24deac-ed68-41c2-ba52-d7557909c556
📒 Files selected for processing (33)
RemuxApp/Sources/Domain/TerminalSettings.swiftRemuxApp/Sources/Ghostty/GhosttyIOSurfaceFrame.swiftRemuxApp/Sources/Ghostty/GhosttyKeyboardChrome.swiftRemuxApp/Sources/Ghostty/GhosttyKitControlSurface.swiftRemuxApp/Sources/Ghostty/GhosttyKitRuntime.swiftRemuxApp/Sources/Ghostty/GhosttyManagedSurface.swiftRemuxApp/Sources/Ghostty/GhosttyPanePreviewSession.swiftRemuxApp/Sources/Ghostty/GhosttyPaneScrollContainerView.swiftRemuxApp/Sources/Ghostty/GhosttySingleViewportView.swiftRemuxApp/Sources/Ghostty/GhosttySurfaceScreen.swiftRemuxApp/Sources/Ghostty/GhosttySurfaceSelectionSheet.swiftRemuxApp/Sources/Ghostty/GhosttyTerminalPresentationProjector.swiftRemuxApp/Sources/Ghostty/GhosttyTerminalScreenModeling.swiftRemuxApp/Sources/Ghostty/PanePreviewLayout.swiftRemuxApp/Sources/Ghostty/TerminalSelectionSheetStyle.swiftRemuxApp/Sources/Tmux/TmuxPanePreviewImageCache.swiftRemuxApp/Sources/Tmux/TmuxPaneSurface.swiftRemuxApp/Sources/Tmux/TmuxScreenModel.swiftRemuxApp/Sources/Tmux/TmuxSessionController.swiftRemuxApp/Sources/Tmux/TmuxSessionLink.swiftRemuxApp/Sources/Tmux/TmuxTerminalScreenAdapter.swiftRemuxApp/Sources/Tmux/TmuxTerminalSession.swiftRemuxAppTests/GhosttyKitControlSurfaceTests.swiftRemuxAppTests/GhosttyKitRuntimeTests.swiftRemuxAppTests/GhosttyPanePreviewSessionTests.swiftRemuxAppTests/GhosttyTerminalPresentationProjectorTests.swiftRemuxAppTests/PanePreviewLayoutTests.swiftRemuxAppTests/TmuxSessionControllerClientSizeTests.swiftRemuxAppTests/TmuxSessionLinkWriteFailureTests.swiftRemuxAppTests/TmuxTerminalScreenAdapterTests.swiftRemuxAppTests/TmuxTerminalSessionShutdownDrainTests.swiftRemuxAppUITests/RemuxAppUITests.swiftscripts/fetch_ghosttykit.sh
💤 Files with no reviewable changes (1)
- RemuxAppTests/TmuxTerminalSessionShutdownDrainTests.swift
| let viewportPanesByID = Dictionary( | ||
| uniqueKeysWithValues: (viewport?.panes ?? []).map { ($0.id, $0) } | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Avoid a trapping dictionary initializer for viewport panes.
Dictionary(uniqueKeysWithValues:) traps at runtime if two entries in viewport.panes share an id. paneSelectionSheetRenderProjection is a static entry point that accepts any caller-supplied projection, so a duplicate pane identity crashes the sheet instead of degrading to a missing frame. Use Dictionary(_:uniquingKeysWith:) so the projection stays defined for every input.
🛡️ Proposed fix
- let viewportPanesByID = Dictionary(
- uniqueKeysWithValues: (viewport?.panes ?? []).map { ($0.id, $0) }
- )
+ let viewportPanesByID = Dictionary(
+ (viewport?.panes ?? []).map { ($0.id, $0) },
+ uniquingKeysWith: { first, _ in first }
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let viewportPanesByID = Dictionary( | |
| uniqueKeysWithValues: (viewport?.panes ?? []).map { ($0.id, $0) } | |
| ) | |
| let viewportPanesByID = Dictionary( | |
| (viewport?.panes ?? []).map { ($0.id, $0) }, | |
| uniquingKeysWith: { first, _ in first } | |
| ) |
🤖 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 `@RemuxApp/Sources/Ghostty/GhosttyTerminalPresentationProjector.swift` around
lines 730 - 732, Update the viewportPanesByID construction in
paneSelectionSheetRenderProjection to use the non-trapping Dictionary
initializer with a duplicate-key resolution closure, preserving one pane for
each ID instead of crashing when viewport.panes contains duplicates.
| private func handleActivePaneChanged(_ paneID: TmuxPaneID) { | ||
| guard paneSurface?.paneID == paneID else { return } | ||
| paneSurface?.refreshInteractionState() | ||
| surfacesByPaneID[paneID]?.refreshInteractionState() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the controller publishes topology alongside active-pane changes.
set -euo pipefail
rg -n -C 6 'onActivePaneChanged' RemuxApp/Sources/Tmux/TmuxSessionController.swift
rg -n -C 4 'activePaneID' RemuxApp/Sources/Tmux/TmuxSessionController.swiftRepository: h3nock/remux
Length of output: 7085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant terminal session handling without running repository code.
printf '--- TmuxTerminalSession outline ---\n'
ast-grep outline RemuxApp/Sources/Tmux/TmuxTerminalSession.swift || true
printf '\n--- Relevant TmuxTerminalSession sections ---\n'
sed -n '1,140p' RemuxApp/Sources/Tmux/TmuxTerminalSession.swift
printf '\n--- Section around 200-290 ---\n'
sed -n '200,290p' RemuxApp/Sources/Tmux/TmuxTerminalSession.swift
printf '\n--- Section around 140-220 for reconcilePresentationActivity ---\n'
rg -n -C 12 'reconcilePresentationActivity|focus|display|activePaneID|topology' RemuxApp/Sources/Tmux/TmuxTerminalSession.swift
printf '\n--- Relevant TmuxSessionController topology callbacks ---\n'
sed -n '1280,1420p' RemuxApp/Sources/Tmux/TmuxSessionController.swiftRepository: h3nock/remux
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
controller = Path("RemuxApp/Sources/Tmux/TmuxSessionController.swift").read_text()
session = Path("RemuxApp/Sources/Tmux/TmuxTerminalSession.swift").read_text()
# Extract callback publication snippets around onTopology and onActivePaneChanged.
for name in ["onTopology", "onActivePaneChanged"]:
print(f"--- {name} occurrences ---")
for m in re.finditer(rf'self\.callbacks\.{name}[^;\n]*;', controller):
start=max(0, controller.rfind("\n", 0, m.start()) - 1200)
end=min(len(controller), controller.find("\n", m.end()) + 800)
print(controller[start:end])
print()
# Behavioral/data-shape check: whether each activePaneChanged publication site in refreshStateByPaneID
# is textually preceded by a topology callback publication in the same function.
for match in re.finditer(r'func handle.*refreshStateByPaneID\s*:', controller):
func_start = match.start()
func_end = controller.find("\n}\n", func_start)
if func_end == -1:
func_end = len(controller)
body = controller[func_start:func_end]
on_topology = [controller.count(s, 0, i) for i, s in enumerate(body) if s == "\n"] # placeholder/no-op
print("refreshStateByPaneID function section present:", bool(body))
if body:
print(body[:800])
print("contains onTopology call:", bool(re.search(r'self\.callbacks\.onTopology\s*\(', body)))
if "activePaneID(in: topology) == paneID" in body:
print("contains activePaneID(in: topology) == paneID:", True)
print("Terminal session handles reference topology?", "topology" in session)
print("Terminal session sets focused from reconcilePresentationActivity?", "setFocused(true)" in session)
PYRepository: h3nock/remux
Length of output: 343
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- TmuxTerminalSession outline ---"
ast-grep outline RemuxApp/Sources/Tmux/TmuxTerminalSession.swift || true
echo
echo "--- TmuxTerminalSession relevant sections ---"
sed -n '1,140p' RemuxApp/Sources/Tmux/TmuxTerminalSession.swift
sed -n '140,290p' RemuxApp/Sources/Tmux/TmuxTerminalSession.swift
echo
echo "--- TmuxSessionController active-pane callback publication sites ---"
rg -n -C 8 'callbacks\.onActivePaneChanged|callbacks\.onTopology' RemuxApp/Sources/Tmux/TmuxSessionController.swift
echo
echo "--- TmuxSessionController refreshStateByPaneID helper sections ---"
rg -n -C 25 'func handle.*refreshStateByPaneID|activePaneID\(in: topology\) == paneID|onTopology' RemuxApp/Sources/Tmux/TmuxSessionController.swiftRepository: h3nock/remux
Length of output: 21374
Reconcile focus in handleActivePaneChanged.
onActivePaneChanged is only used by handleActivePaneChanged, which refreshes interaction state. Display and keyboard focus are reconciled by reconcilePresentationActivity through handleTopology; if native active-pane events are not always accompanied by a topology callback, the currently focused pane stays focused without any focus recipient being told it stopped receiving focus.
🤖 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 `@RemuxApp/Sources/Tmux/TmuxTerminalSession.swift` around lines 256 - 258, The
handleActivePaneChanged method must reconcile presentation activity when the
native active-pane event occurs, rather than only refreshing the affected
surface’s interaction state. Update handleActivePaneChanged to invoke the
existing reconcilePresentationActivity flow (using the current topology/state
inputs) so display and keyboard focus are reassigned consistently even without a
topology callback.
| private func removePanePickerItem(_ tile: XCUIElement) { | ||
| let tilePrefix = "terminal.pane.tile." | ||
| XCTAssertTrue(tile.identifier.hasPrefix(tilePrefix)) | ||
| let paneIdentifier = String(tile.identifier.dropFirst(tilePrefix.count)) | ||
| XCTAssertFalse(paneIdentifier.isEmpty) | ||
|
|
||
| tile.press(forDuration: 1.0) | ||
| tapPickerButton( | ||
| identifier: "terminal.pane.remove.\(paneIdentifier)", | ||
| fallbackLabel: "Remove Pane" | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect pane removal identifiers, labels, and any confirmation control in the selection sheet.
set -uo pipefail
fd -t f 'GhosttySurfaceSelectionSheet.swift' --exec rg -n -C 6 'terminal\.pane\.(tile|remove)|Remove Pane|remove\.confirm' {} \;
rg -n -C 4 'terminal\.pane\.remove|terminal\.pane\.tile' --glob '*.swift'Repository: h3nock/remux
Length of output: 3300
🏁 Script executed:
#!/bin/bash
# Read the relevant GhosttySurfaceSelectionSheet section without relying on fd's exit status.
set -u
file="$(fd -t f 'GhosttySurfaceSelectionSheet.swift' | head -n 1)"
if [ -n "${file:-}" ]; then
sed -n '300,360p' "$file" | cat -n
sed -n '255,284p' "$file" | cat -n
fiRepository: h3nock/remux
Length of output: 5202
Add the pane removal confirmation tap.
The pane removal action button uses terminal.pane.remove.<pane id> and accessibility label Remove Pane, but it opens a confirmation dialog. Add a tap on terminal.pane.remove.confirm after tapping the removal action.
🤖 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 `@RemuxAppUITests/RemuxAppUITests.swift` around lines 3709 - 3720, The
removePanePickerItem helper currently stops after opening the pane removal
confirmation dialog. After tapPickerButton invokes terminal.pane.remove.<pane
id> or its fallback label, add a tap for terminal.pane.remove.confirm to
complete the confirmed removal.
1 participant
Summary
Render tmux windows as live multi-pane canvases in Remux and add pane-aware controls, previews, input routing, zoom handling, and deletion reliability.
Previously, Remux handled multipane windows by automatically zooming the active tmux pane and presenting only that pane. This change presents the complete pane layout by default and makes tmux zoom an explicit user action.
This change retains one terminal renderer per hydrated pane and composes those renderers using the authoritative geometry reported by tmux.
What changed
Multi-pane presentation
Pane controls
Window controls
Zoom behavior
Remux makes a best-effort cleanup attempt for zooms it created when the actual Remux session shuts down. iOS does not guarantee execution when the app is force-killed, so server zoom can remain in that case and may need to be cleared from another tmux client using the tmux prefix followed by
z—usuallyCtrl-b, thenz.Preview capture
Validation completed
Screenshots
Summary by CodeRabbit
New Features
Improvements