Fix Ghostty resize_split keybind support - #1899
Merged
lawrencecchen merged 7 commits intoMar 25, 2026
Merged
Conversation
|
@jorgitin02 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughImplements full logic for TabManager.resizeSplit: validates input, locates the target pane and Bonsplit controller, traverses the ExternalTreeNode to collect candidate split nodes, selects a matching split by orientation and child position, computes a scaled divider delta, clamps it to [0.1, 0.9], and applies the divider update. Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller
participant TabMgr as TabManager
participant BonsMgr as BonsplitController
participant Tree as ExternalTreeNode
participant State as SplitState
Caller->>TabMgr: resizeSplit(tabId, surfaceId, direction, amount)
TabMgr->>TabMgr: validate amount > 0
TabMgr->>BonsMgr: resolve pane & controller for surfaceId
TabMgr->>Tree: snapshot & traverse ExternalTreeNode (recursive)
Tree-->>TabMgr: candidate split nodes containing pane
TabMgr->>TabMgr: filter by direction.splitOrientation & child position
TabMgr->>TabMgr: compute divider delta (direction.dividerDeltaSign * amount / axisPixels)
TabMgr->>TabMgr: clamp newPos to [0.1, 0.9]
TabMgr->>State: setDividerPosition(newPos, fromExternal: true)
State-->>TabMgr: update result
TabMgr-->>Caller: return true/false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Contributor
Greptile SummaryThis PR implements the previously stubbed-out Key observations:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["resizeSplit(tabId, surfaceId, direction, amount)"] --> B{amount > 0 &&\ntab found &&\npaneId found?}
B -- No --> Z[return false]
B -- Yes --> C{paneUUID in\nallPaneIds?}
C -- No --> Z
C -- Yes --> D["resizeSplitCollectCandidates(treeSnapshot)"]
D --> E{containsTarget?}
E -- No --> Z
E -- Yes --> F["filter candidates by\ndirection.splitOrientation"]
F --> G{any matches?}
G -- No --> Z
G -- Yes --> H["find first where\npaneInFirstChild ==\ndirection.requiresPaneInFirstChild"]
H -- No match --> Z
H -- Match found --> I["delta = amount / axisPixels\nrequested = dividerPos + sign × delta"]
I --> J["clamped = min(max(requested, 0.1), 0.9)"]
J --> K["setDividerPosition(clamped, forSplit, fromExternal: true)"]
K --> L[return Bool result]
subgraph "resizeSplitCollectCandidates (post-order)"
P1[".pane node"] --> P2["return Trace(containsTarget: id==target, bounds: frame)"]
S1[".split node"] --> S2["recurse into first child"]
S2 --> S3["recurse into second child"]
S3 --> S4{containsTarget?}
S4 -- Yes --> S5["append ResizeSplitCandidate\n(splitId, orientation, paneInFirstChild,\ndividerPos, axisPixels)"]
S5 --> S6["return combined Trace"]
S4 -- No --> S6
end
Last reviewed commit: "fix: implement Ghost..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+3360
to
+3420
| private struct ResizeSplitCandidate { | ||
| let splitId: UUID | ||
| let orientation: String | ||
| let paneInFirstChild: Bool | ||
| let dividerPosition: CGFloat | ||
| let axisPixels: CGFloat | ||
| } | ||
|
|
||
| private struct ResizeSplitTrace { | ||
| let containsTarget: Bool | ||
| let bounds: CGRect | ||
| } | ||
|
|
||
| private func resizeSplitCollectCandidates( | ||
| node: ExternalTreeNode, | ||
| targetPaneId: String, | ||
| candidates: inout [ResizeSplitCandidate] | ||
| ) -> ResizeSplitTrace { | ||
| switch node { | ||
| case .pane(let pane): | ||
| let bounds = CGRect( | ||
| x: pane.frame.x, | ||
| y: pane.frame.y, | ||
| width: pane.frame.width, | ||
| height: pane.frame.height | ||
| ) | ||
| return ResizeSplitTrace(containsTarget: pane.id == targetPaneId, bounds: bounds) | ||
|
|
||
| case .split(let split): | ||
| let first = resizeSplitCollectCandidates( | ||
| node: split.first, | ||
| targetPaneId: targetPaneId, | ||
| candidates: &candidates | ||
| ) | ||
| let second = resizeSplitCollectCandidates( | ||
| node: split.second, | ||
| targetPaneId: targetPaneId, | ||
| candidates: &candidates | ||
| ) | ||
|
|
||
| let combinedBounds = first.bounds.union(second.bounds) | ||
| let containsTarget = first.containsTarget || second.containsTarget | ||
|
|
||
| if containsTarget, | ||
| let splitUUID = UUID(uuidString: split.id) { | ||
| let orientation = split.orientation.lowercased() | ||
| let axisPixels: CGFloat = orientation == "horizontal" | ||
| ? combinedBounds.width | ||
| : combinedBounds.height | ||
| candidates.append(ResizeSplitCandidate( | ||
| splitId: splitUUID, | ||
| orientation: orientation, | ||
| paneInFirstChild: first.containsTarget, | ||
| dividerPosition: CGFloat(split.dividerPosition), | ||
| axisPixels: max(axisPixels, 1) | ||
| )) | ||
| } | ||
|
|
||
| return ResizeSplitTrace(containsTarget: containsTarget, bounds: combinedBounds) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
TerminalController
ResizeSplitCandidate, ResizeSplitTrace, and resizeSplitCollectCandidates are near-identical copies of V2PaneResizeCandidate, V2PaneResizeTrace, and v2PaneResizeCollectCandidates (lines ~5853–5944 in TerminalController.swift). Similarly, the new splitOrientation, requiresPaneInFirstChild, and dividerDeltaSign properties on ResizeDirection are functionally identical to those on the private V2PaneResizeDirection enum.
Now that TabManager.resizeSplit is correctly implemented, TerminalController.v2PaneResize could be simplified to call it (mapping V2PaneResizeDirection to ResizeDirection), or both could share a common internal helper. Leaving these two paths to diverge independently will make future changes (e.g., changing clamp bounds, fixing a traversal bug) error-prone — they'd need to be applied in two places.
Contributor
Author
There was a problem hiding this comment.
Keeping the refactor out of this PR to keep the fix scope narrow. I addressed the missing coverage feedback in follow-up test commit 75c94247; if maintainers want the shared-helper consolidation, I’d treat that as a separate cleanup change.
Contributor
There was a problem hiding this comment.
Agree this is worth consolidating. Keeping it out of this PR to avoid scope creep. The merge-with-main push (8c5eec0) resolves the conflict; a follow-up cleanup PR can unify the two paths.
Comment on lines
747
to
934
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| final class TabManagerResizeSplitsTests: XCTestCase { | ||
| func testResizeSplitMovesHorizontalDividerRightForFirstChildPane() { | ||
| let manager = TabManager() | ||
| guard let workspace = manager.selectedWorkspace, | ||
| let leftPanelId = workspace.focusedPanelId, | ||
| workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal) != nil else { | ||
| XCTFail("Expected split setup to succeed") | ||
| return | ||
| } | ||
|
|
||
| guard let split = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first, | ||
| let splitId = UUID(uuidString: split.id) else { | ||
| XCTFail("Expected a split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertTrue( | ||
| workspace.bonsplitController.setDividerPosition(0.5, forSplit: splitId), | ||
| "Expected to seed divider position" | ||
| ) | ||
|
|
||
| XCTAssertTrue( | ||
| manager.resizeSplit(tabId: workspace.id, surfaceId: leftPanelId, direction: .right, amount: 120), | ||
| "Expected resizeSplit to succeed for the right edge of the left pane" | ||
| ) | ||
|
|
||
| guard let updatedSplit = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected updated split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertGreaterThan( | ||
| updatedSplit.dividerPosition, | ||
| 0.5, | ||
| "Expected resizing the left pane to the right to move the divider toward the second child" | ||
| ) | ||
| } | ||
|
|
||
| func testResizeSplitMovesHorizontalDividerLeftForSecondChildPane() { | ||
| let manager = TabManager() | ||
| guard let workspace = manager.selectedWorkspace, | ||
| let leftPanelId = workspace.focusedPanelId, | ||
| let rightPanel = workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal) else { | ||
| XCTFail("Expected split setup to succeed") | ||
| return | ||
| } | ||
|
|
||
| guard let split = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first, | ||
| let splitId = UUID(uuidString: split.id) else { | ||
| XCTFail("Expected a split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertTrue( | ||
| workspace.bonsplitController.setDividerPosition(0.5, forSplit: splitId), | ||
| "Expected to seed divider position" | ||
| ) | ||
|
|
||
| XCTAssertTrue( | ||
| manager.resizeSplit(tabId: workspace.id, surfaceId: rightPanel.id, direction: .left, amount: 120), | ||
| "Expected resizeSplit to succeed for the left edge of the right pane" | ||
| ) | ||
|
|
||
| guard let updatedSplit = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected updated split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertLessThan( | ||
| updatedSplit.dividerPosition, | ||
| 0.5, | ||
| "Expected resizing the right pane to the left to move the divider toward the first child" | ||
| ) | ||
| } | ||
|
|
||
| func testResizeSplitMovesVerticalDividerDownForFirstChildPane() { | ||
| let manager = TabManager() | ||
| guard let workspace = manager.selectedWorkspace, | ||
| let topPanelId = workspace.focusedPanelId, | ||
| workspace.newTerminalSplit(from: topPanelId, orientation: .vertical) != nil else { | ||
| XCTFail("Expected split setup to succeed") | ||
| return | ||
| } | ||
|
|
||
| guard let split = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first, | ||
| let splitId = UUID(uuidString: split.id) else { | ||
| XCTFail("Expected a split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertTrue( | ||
| workspace.bonsplitController.setDividerPosition(0.5, forSplit: splitId), | ||
| "Expected to seed divider position" | ||
| ) | ||
|
|
||
| XCTAssertTrue( | ||
| manager.resizeSplit(tabId: workspace.id, surfaceId: topPanelId, direction: .down, amount: 120), | ||
| "Expected resizeSplit to succeed for the bottom edge of the top pane" | ||
| ) | ||
|
|
||
| guard let updatedSplit = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected updated split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertGreaterThan( | ||
| updatedSplit.dividerPosition, | ||
| 0.5, | ||
| "Expected resizing the top pane downward to move the divider toward the second child" | ||
| ) | ||
| } | ||
|
|
||
| func testResizeSplitReturnsFalseWhenPaneHasNoBorderInDirection() { | ||
| let manager = TabManager() | ||
| guard let workspace = manager.selectedWorkspace, | ||
| let leftPanelId = workspace.focusedPanelId, | ||
| workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal) != nil else { | ||
| XCTFail("Expected split setup to succeed") | ||
| return | ||
| } | ||
|
|
||
| guard let split = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected a split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertFalse( | ||
| manager.resizeSplit(tabId: workspace.id, surfaceId: leftPanelId, direction: .left, amount: 120), | ||
| "Expected resizeSplit to fail when the pane has no adjacent border in that direction" | ||
| ) | ||
|
|
||
| guard let updatedSplit = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected updated split node in tree snapshot") | ||
| return | ||
| } | ||
| XCTAssertEqual(updatedSplit.dividerPosition, split.dividerPosition, accuracy: 0.000_1) | ||
| } | ||
|
|
||
| func testResizeSplitClampsDividerPositionAtUpperBound() { | ||
| let manager = TabManager() | ||
| guard let workspace = manager.selectedWorkspace, | ||
| let leftPanelId = workspace.focusedPanelId, | ||
| workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal) != nil else { | ||
| XCTFail("Expected split setup to succeed") | ||
| return | ||
| } | ||
|
|
||
| guard let split = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first, | ||
| let splitId = UUID(uuidString: split.id) else { | ||
| XCTFail("Expected a split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertTrue( | ||
| workspace.bonsplitController.setDividerPosition(0.89, forSplit: splitId), | ||
| "Expected to seed divider position near upper bound" | ||
| ) | ||
|
|
||
| XCTAssertTrue( | ||
| manager.resizeSplit(tabId: workspace.id, surfaceId: leftPanelId, direction: .right, amount: 10_000), | ||
| "Expected resizeSplit to clamp instead of failing" | ||
| ) | ||
|
|
||
| guard let updatedSplit = splitNodes(in: workspace.bonsplitController.treeSnapshot()).first else { | ||
| XCTFail("Expected updated split node in tree snapshot") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertEqual(updatedSplit.dividerPosition, 0.9, accuracy: 0.000_1) | ||
| } | ||
|
|
||
| private func splitNodes(in node: ExternalTreeNode) -> [ExternalSplitNode] { | ||
| switch node { | ||
| case .pane: | ||
| return [] | ||
| case .split(let split): | ||
| return [split] + splitNodes(in: split.first) + splitNodes(in: split.second) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| @MainActor | ||
| final class TabManagerWorkspaceConfigInheritanceSourceTests: XCTestCase { |
Contributor
There was a problem hiding this comment.
up-direction coverage
The test suite covers right (first-child horizontal), left (second-child horizontal), down (first-child vertical), the no-border guard, and upper-bound clamping, but two cases are absent:
-
Lower clamp bound — there is no symmetric counterpart to
testResizeSplitClampsDividerPositionAtUpperBoundthat seeds a position near0.1and applies a large negative delta, verifying the result is clamped to0.1. The upper- and lower-bound code paths are separatemin/maxcalls and should each be exercised. -
updirection — no test exercises the second-child vertical case (resize_split:up).downcoversfirst.containsTarget && orientation == "vertical", butsecond.containsTarget && orientation == "vertical"(i.e.,requiresPaneInFirstChild == falsefor a vertical split) is untested.
Contributor
Author
There was a problem hiding this comment.
Addressed in 75c94247 by adding focused coverage for the missing resize_split:up path and the lower clamp bound in TabManagerResizeSplitsTests. Verified locally with the focused split test slice after the change.
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmuxTests/TabManagerUnitTests.swift (1)
750-930: Strengthen this suite with one nested exact-delta case.These cases currently prove sign/clamping on one-split layouts, but the risky logic is ancestor selection in nested trees. A regression that moves the wrong split—or scales
amountincorrectly—could still pass here. Adding one 2x2 or 3-column case that asserts which split moved and by how much would lock down the new behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 750 - 930, Add a new unit test (e.g., testResizeSplitMovesCorrectAncestorInNestedTree) that builds a nested split layout (create a split, then split one child again to produce a 2x2 or 3-column arrangement), seed known divider positions via workspace.bonsplitController.setDividerPosition for each split, capture the specific split nodes using the existing splitNodes(in:) helper, call manager.resizeSplit(tabId:workspace.id, surfaceId:<innerPaneId>, direction:<dir>, amount:<exact amount>), and then assert (1) only the correct ancestor split’s dividerPosition changed and (2) the change equals the expected delta or clamped value while other splits’ dividerPosition remain equal to their pre-resize values within a tight accuracy. Ensure you reference manager.resizeSplit, workspace.newTerminalSplit, bonsplitController.setDividerPosition and splitNodes(in:) so the test targets the ancestor-selection logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 750-930: Add a new unit test (e.g.,
testResizeSplitMovesCorrectAncestorInNestedTree) that builds a nested split
layout (create a split, then split one child again to produce a 2x2 or 3-column
arrangement), seed known divider positions via
workspace.bonsplitController.setDividerPosition for each split, capture the
specific split nodes using the existing splitNodes(in:) helper, call
manager.resizeSplit(tabId:workspace.id, surfaceId:<innerPaneId>,
direction:<dir>, amount:<exact amount>), and then assert (1) only the correct
ancestor split’s dividerPosition changed and (2) the change equals the expected
delta or clamped value while other splits’ dividerPosition remain equal to their
pre-resize values within a tight accuracy. Ensure you reference
manager.resizeSplit, workspace.newTerminalSplit,
bonsplitController.setDividerPosition and splitNodes(in:) so the test targets
the ancestor-selection logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fec44624-fd45-4a74-87e8-cff8305509cf
📒 Files selected for processing (2)
Sources/TabManager.swiftcmuxTests/TabManagerUnitTests.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmuxTests/TabManagerUnitTests.swift (1)
992-999: Consider extracting shared helper to file scope.The
splitNodes(in:)helper is duplicated betweenTabManagerEqualizeSplitsTests(lines 740-747) and this class. A file-level private function would reduce duplication.♻️ Proposed refactor to consolidate helper
Add at file scope (e.g., near line 25):
private func splitNodes(in node: ExternalTreeNode) -> [ExternalSplitNode] { switch node { case .pane: return [] case .split(let split): return [split] + splitNodes(in: split.first) + splitNodes(in: split.second) } }Then remove the private method from both
TabManagerEqualizeSplitsTestsandTabManagerResizeSplitsTests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 992 - 999, Extract the duplicated helper splitNodes(in:) out of the two test classes into a single file-scoped private function (e.g., near the top of the test file) and remove the duplicate private methods from TabManagerEqualizeSplitsTests and TabManagerResizeSplitsTests (or the current class). Keep the implementation identical and ensure it remains private to the file so both classes can call splitNodes(in:) directly without changing call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 992-999: Extract the duplicated helper splitNodes(in:) out of the
two test classes into a single file-scoped private function (e.g., near the top
of the test file) and remove the duplicate private methods from
TabManagerEqualizeSplitsTests and TabManagerResizeSplitsTests (or the current
class). Keep the implementation identical and ensure it remains private to the
file so both classes can call splitNodes(in:) directly without changing call
sites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93061747-c11e-493d-b57b-4ed6765c3c2e
📒 Files selected for processing (1)
cmuxTests/TabManagerUnitTests.swift
Contributor
Author
|
Addressed the remaining helper-dedup nit in I moved |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmuxTests/TabManagerUnitTests.swift (1)
750-991: Add one nested-layout regression test for ancestor split selection.Current coverage is strong for single-split direction/no-op/clamp behavior, but it does not directly assert the nested case where
resizeSplitmust choose the matching ancestor split by orientation + pane edge. That’s the key Ghostty regression path.Suggested test addition
`@MainActor` final class TabManagerResizeSplitsTests: XCTestCase { + func testResizeSplitTargetsMatchingAncestorInNestedLayout() { + let manager = TabManager() + guard let workspace = manager.selectedWorkspace, + let leftPanelId = workspace.focusedPanelId, + let rightPanel = workspace.newTerminalSplit(from: leftPanelId, orientation: .horizontal), + let bottomRightPanel = workspace.newTerminalSplit(from: rightPanel.id, orientation: .vertical) else { + XCTFail("Expected nested split setup to succeed") + return + } + + let initial = splitNodes(in: workspace.bonsplitController.treeSnapshot()) + guard let horizontal = initial.first(where: { $0.orientation.lowercased() == "horizontal" }), + let vertical = initial.first(where: { $0.orientation.lowercased() == "vertical" }), + let horizontalId = UUID(uuidString: horizontal.id), + let verticalId = UUID(uuidString: vertical.id) else { + XCTFail("Expected both horizontal and vertical split nodes") + return + } + + XCTAssertTrue(workspace.bonsplitController.setDividerPosition(0.5, forSplit: horizontalId)) + XCTAssertTrue(workspace.bonsplitController.setDividerPosition(0.5, forSplit: verticalId)) + + XCTAssertTrue( + manager.resizeSplit(tabId: workspace.id, surfaceId: bottomRightPanel.id, direction: .left, amount: 120), + "Expected resize to target the horizontal ancestor controlling the pane's left edge" + ) + + let updated = splitNodes(in: workspace.bonsplitController.treeSnapshot()) + guard let updatedHorizontal = updated.first(where: { $0.id == horizontal.id }), + let updatedVertical = updated.first(where: { $0.id == vertical.id }) else { + XCTFail("Expected updated split nodes") + return + } + + XCTAssertLessThan(updatedHorizontal.dividerPosition, 0.5) + XCTAssertEqual(updatedVertical.dividerPosition, 0.5, accuracy: 0.000_1) + } + func testResizeSplitMovesHorizontalDividerRightForFirstChildPane() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 750 - 991, Add a nested-layout regression test in TabManagerResizeSplitsTests that sets up a two-level split (create a primary split via workspace.newTerminalSplit(from: focusedPanelId, orientation: .horizontal) and then split one of those children again with the opposite orientation), seed the ancestor split's dividerPosition using workspace.bonsplitController.setDividerPosition(...), then call manager.resizeSplit(tabId: workspace.id, surfaceId: <deeply nested child panel id>, direction: <edge matching ancestor split>, amount: <large positive number>) and assert that the ancestor split (found via splitNodes(in: workspace.bonsplitController.treeSnapshot())) has its dividerPosition moved/clamped as expected; reference TabManager.resizeSplit, workspace.newTerminalSplit, workspace.bonsplitController.setDividerPosition, and splitNodes to locate and implement the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 750-991: Add a nested-layout regression test in
TabManagerResizeSplitsTests that sets up a two-level split (create a primary
split via workspace.newTerminalSplit(from: focusedPanelId, orientation:
.horizontal) and then split one of those children again with the opposite
orientation), seed the ancestor split's dividerPosition using
workspace.bonsplitController.setDividerPosition(...), then call
manager.resizeSplit(tabId: workspace.id, surfaceId: <deeply nested child panel
id>, direction: <edge matching ancestor split>, amount: <large positive number>)
and assert that the ancestor split (found via splitNodes(in:
workspace.bonsplitController.treeSnapshot())) has its dividerPosition
moved/clamped as expected; reference TabManager.resizeSplit,
workspace.newTerminalSplit, workspace.bonsplitController.setDividerPosition, and
splitNodes to locate and implement the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5974d6fa-1d48-4b5a-88bc-fe68fd3ccbaf
📒 Files selected for processing (1)
cmuxTests/TabManagerUnitTests.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/TabManager.swift (1)
3690-3701: Consider adding documentation to these helper types.These internal types are clear from context, but a brief doc comment explaining their purpose would help future maintainers understand the resize_split flow at a glance.
📝 Example documentation
+ /// Metadata about a split node that is a candidate for resize operations. + /// Collected during tree traversal when the split's subtree contains the target pane. private struct ResizeSplitCandidate { let splitId: UUID let orientation: String let paneInFirstChild: Bool let dividerPosition: CGFloat let axisPixels: CGFloat } + /// Result of traversing an ExternalTreeNode subtree during resize candidate collection. private struct ResizeSplitTrace { let containsTarget: Bool let bounds: CGRect }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TabManager.swift` around lines 3690 - 3701, Add concise doc comments above the two internal structs to explain their roles in the resize_split flow: document that ResizeSplitCandidate represents a potential split divider to be adjusted (fields: splitId, orientation, paneInFirstChild, dividerPosition, axisPixels) and what each field means, and document that ResizeSplitTrace captures whether a hit-test contains the resize target and the bounds used for that trace (fields: containsTarget, bounds); place these comments immediately above the ResizeSplitCandidate and ResizeSplitTrace declarations so maintainers can quickly understand their purpose when reading TabManager.swift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/TabManager.swift`:
- Around line 3690-3701: Add concise doc comments above the two internal structs
to explain their roles in the resize_split flow: document that
ResizeSplitCandidate represents a potential split divider to be adjusted
(fields: splitId, orientation, paneInFirstChild, dividerPosition, axisPixels)
and what each field means, and document that ResizeSplitTrace captures whether a
hit-test contains the resize target and the bounds used for that trace (fields:
containsTarget, bounds); place these comments immediately above the
ResizeSplitCandidate and ResizeSplitTrace declarations so maintainers can
quickly understand their purpose when reading TabManager.swift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 183b5386-8bcd-4cc0-a603-f5be8750f170
📒 Files selected for processing (2)
Sources/TabManager.swiftcmuxTests/TabManagerUnitTests.swift
Contributor
|
Thank you for the contribution! |
Jesssullivan
added a commit
to Jesssullivan/cmux
that referenced
this pull request
Mar 26, 2026
Ingests all upstream fixes since 2026-03-22 including: - Fix Cmd+N crash: retain snapshot workspaces (manaflow-ai#2183, manaflow-ai#2181, manaflow-ai#2178, manaflow-ai#2173) - Fix browser pane restore after reopen (manaflow-ai#2141) - Fix Ghostty resize_split keybind (manaflow-ai#1899) - Reduce shell integration prompt latency (manaflow-ai#2109) - Fix command palette focus after terminal find (manaflow-ai#2089) - Add Codex CLI hooks (manaflow-ai#2103) - Add cmux.json custom commands (manaflow-ai#2011) - Fix window position restore on relaunch (manaflow-ai#2129) Conflict resolution: - BrowserPanel.swift: accepted upstream configureWebViewConfiguration() refactor (already includes our forMainFrameOnly:true CAPTCHA fix from PR manaflow-ai#1877) Fork-specific files preserved: - Sources/Panels/WebAuthn{Coordinator,BridgeJavaScript}.swift - Sources/FIDO2/module.modulemap - vendor/ctap2 submodule - cmux.entitlements (with camera/audio-input removed) - cmux.embedded.entitlements - .github/workflows/fork-{ci,release}.yml
3 tasks
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
* test: add resize_split regression coverage * fix: implement Ghostty resize_split behavior * test: cover more resize_split cases * test: deduplicate split snapshot helper * Resolve merge conflict: keep both splitNodes and waitForCondition helpers --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
franckrasolo
added a commit
to franckrasolo/dotfiles.nix
that referenced
this pull request
Apr 12, 2026
franckrasolo
added a commit
to franckrasolo/dotfiles.nix
that referenced
this pull request
Jul 24, 2026
franckrasolo
added a commit
to franckrasolo/dotfiles.nix
that referenced
this pull request
Jul 24, 2026
5 of 6 tasks
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.
Summary
TabManager.resizeSplitresize_splitby selecting the matching ancestor split and moving its divider with clamp boundsTest Plan
TOOLCHAINS=MetalToolchain xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-resize-split-red -only-testing:cmuxTests/TabManagerResizeSplitsTests testTOOLCHAINS=MetalToolchain xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-split-tests-pr -only-testing:cmuxTests/TabManagerEqualizeSplitsTests -only-testing:cmuxTests/TabManagerResizeSplitsTests testTabManagerassertion failures reproduced unchanged on baseline atcmuxTests/TabManagerUnitTests.swift:115,cmuxTests/TabManagerUnitTests.swift:116, andcmuxTests/TabManagerUnitTests.swift:599Summary by cubic
Adds proper support for Ghostty
resize_splitkeybinds by moving the correct ancestor divider for the targeted pane, with clamped bounds. Fixes prior no-op behavior so keybinds resize the expected pane edge across nested splits (addresses issue 1882).Bug Fixes
TabManager.resizeSplitto find the matching ancestor split (by orientation and pane edge) and move its divider; scales movement by axis pixels, clamps to 0.1–0.9, no-ops if there’s no adjacent border; keeps equalize/adjacent-split behavior unchanged.Refactors
ResizeDirectionutilities; consolidated thesplitNodestest helper.Written for commit 8c5eec0. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests