From a6d7cde97f67cc1f109d58a4452d565e77f273c7 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 02:40:37 -0700 Subject: [PATCH 01/10] Divider drags are sessions; imposed splits stop fighting foreign resizes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drag start and end come from the divider's mouse-tracking lifecycle (mouseDown brackets AppKit's tracking loop), not from which event happens to be current inside a resize callback — the old inference missed any drag that paused before release. BonsplitController.isDividerDragActive and the dragDidBegin/End delegate calls expose the session to hosts. An imposed split re-applies its extent only on a fresh imposition, or when the divider moved while the split view's size did not — putting a drifted divider back where nothing else will. Re-asserting against a resized container fought AppKit from inside its own layout pass, recursively; a main-thread sample showed the whole thread in that fight. When a refused parent apply lands on a later retry, nested imposed splits re-apply against their settled containers. --- .../Controllers/SplitViewController.swift | 12 ++ .../Internal/Views/SplitContainerView.swift | 173 +++++++++++++++--- .../Bonsplit/Public/BonsplitController.swift | 24 +++ .../Bonsplit/Public/BonsplitDelegate.swift | 13 ++ 4 files changed, 197 insertions(+), 25 deletions(-) diff --git a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift index 35b742dc..3e4262c6 100644 --- a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift +++ b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift @@ -70,6 +70,18 @@ final class SplitViewController { /// Callback for geometry changes var onGeometryChange: (() -> Void)? + /// Live divider drag sessions across every split in this tree (0 or 1 in + /// practice — AppKit tracks one divider at a time). Sessions bracket the + /// divider's mouse-tracking lifecycle, so external sizing can consult + /// this before writing geometry: mid-drag the user owns the divider. + @ObservationIgnored private(set) var activeDividerDragSessions = 0 + @ObservationIgnored var onDividerDragSessionChange: ((Bool) -> Void)? + + func noteDividerDragSession(_ active: Bool) { + activeDividerDragSessions = max(0, activeDividerDragSessions + (active ? 1 : -1)) + onDividerDragSessionChange?(active) + } + init(rootNode: SplitNode? = nil) { let resolvedRoot: SplitNode let initialFocusedPaneId: PaneID? diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index ac2671cb..2e7ef88d 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -97,6 +97,55 @@ private class ThemedSplitView: NSSplitView, BonsplitManagedSplitView { // See `NonDraggableHostingView` in SplitNodeView.swift for the rest of // the chain. override var mouseDownCanMoveWindow: Bool { false } + + /// Brackets a divider drag as a session: fires `true` when a mouseDown + /// lands on the divider's effective hit rect, `false` when AppKit's + /// divider tracking loop returns at mouseUp. Deterministic — taken from + /// the mouse lifecycle itself, never inferred from which event happens + /// to be current when a resize callback fires (that inference misses a + /// drag-pause-release, where no resize coincides with the mouseUp). + var onDividerDragSession: ((Bool) -> Void)? + + private func dividerSessionHitRect() -> NSRect? { + // No minimum-size guards here, deliberately: a pane parked at its + // 1pt minimum is exactly the pane a user grabs the divider to + // restore, and AppKit still tracks that drag (the delegate's + // effectiveRect has no such guard either). A session that fails to + // arm there would let sizing passes impose over the live drag. + guard arrangedSubviews.count >= 2 else { return nil } + let a = arrangedSubviews[0].frame + let thickness = dividerThickness + let rect: NSRect + if isVertical { + rect = NSRect(x: max(0, a.maxX), y: 0, width: thickness, height: bounds.height) + } else { + rect = NSRect(x: 0, y: max(0, a.maxY), width: bounds.width, height: thickness) + } + return rect.insetBy(dx: -resolvedDividerHitExpansion, dy: -resolvedDividerHitExpansion) + } + + override func mouseDown(with event: NSEvent) { + let location = convert(event.locationInWindow, from: nil) + // Max-edge INCLUSIVE, matching the delegate's dividerHitRectContains: + // AppKit can report a point exactly on the divider boundary, and an + // exclusive contains() would miss the session while NSSplitView still + // tracks the drag. + let inDivider = dividerSessionHitRect().map { rect in + location.x >= rect.minX && location.x <= rect.maxX + && location.y >= rect.minY && location.y <= rect.maxY + } == true +#if DEBUG + if inDivider { + dlog("divider.session.mouseDown loc=\(Int(location.x)),\(Int(location.y))") + } +#endif + if inDivider { onDividerDragSession?(true) } + // For a divider hit, super runs AppKit's tracking loop and returns + // only after the mouse is released — the session end below is the + // guaranteed drag-end signal. + defer { if inDivider { onDividerDragSession?(false) } } + super.mouseDown(with: event) + } } #if DEBUG @@ -247,6 +296,21 @@ struct SplitContainerView: NSViewRepresentabl context.coordinator.secondHostingController = secondController context.coordinator.splitView = splitView + let internalController = controller + splitView.onDividerDragSession = { [weak coordinator = context.coordinator, weak internalController] active in + // Order matters at session END: the counter must drop to zero + // BEFORE the coordinator's final geometry notification, or the + // delegate still reads the drag as live and skips the drag-end + // sync. At BEGIN the coordinator arms first so isDragging is set + // before any delegate reacts to the session. + if active { + coordinator?.dividerDragSessionChanged(true) + internalController?.noteDividerDragSession(true) + } else { + internalController?.noteDividerDragSession(false) + coordinator?.dividerDragSessionChanged(false) + } + } // Capture animation origin before it gets cleared let animationOrigin = splitState.animationOrigin @@ -648,12 +712,12 @@ struct SplitContainerView: NSViewRepresentabl var onGeometryChange: ((_ isDragging: Bool) -> Void)? /// Track last applied position to detect external changes var lastAppliedPosition: CGFloat = 0.5 - /// The imposed-extent memo: the last target we asked AppKit for and - /// the position it actually produced. Re-apply only when the target - /// changes or the divider moved away from that outcome — never in a - /// loop against a target AppKit's constraints refuse to reach. - var lastImposedTarget: CGFloat? + /// What the last imposed apply actually produced, and how big the + /// split view was at the time. syncPosition compares against both to + /// decide whether a moved divider should be put back or left for the + /// host to re-impose at the new size. var lastImposedOutcome: CGFloat? + var lastImposedAvail: CGFloat? var lastImposedEpoch: Int? var imposedRetryBudget = 0 var imposedApplyPending = false @@ -705,8 +769,8 @@ struct SplitContainerView: NSViewRepresentabl splitStateId = newState.id splitState = newState lastAppliedPosition = newState.dividerPosition - lastImposedTarget = nil lastImposedOutcome = nil + lastImposedAvail = nil lastImposedEpoch = nil imposedRetryBudget = 0 didApplyInitialDividerPosition = false @@ -722,6 +786,25 @@ struct SplitContainerView: NSViewRepresentabl splitState = newState } + /// Deterministic drag session, bracketed by `ThemedSplitView.mouseDown` + /// around AppKit's divider tracking loop. Begin arms the drag before + /// any resize callback needs to infer it; end always fires at release + /// and delivers the drag-end geometry notification — whether or not a + /// final resize callback coincided with the mouseUp. The model itself + /// is maintained by the resize callbacks during the drag; end only has + /// to announce that the user's hand is off the divider. + func dividerDragSessionChanged(_ active: Bool) { +#if DEBUG + dlog("divider.session split=\(String(splitState.id.uuidString.prefix(5))) \(active ? "begin" : "end")") +#endif + if active { + isDragging = true + return + } + isDragging = false + onGeometryChange?(false) + } + private func splitTotalSize(in splitView: NSSplitView) -> CGFloat { splitState.orientation == .horizontal ? splitView.bounds.width @@ -847,6 +930,17 @@ struct SplitContainerView: NSViewRepresentabl ? splitView.arrangedSubviews[0].frame.width : splitView.arrangedSubviews[0].frame.height mirrorImposedOutcome(lastImposedOutcome ?? current, in: splitView) + // When this retry finally lands, it resizes everything nested + // inside — AFTER those nested splits already applied their own + // extents and recorded the result. AppKit resizes them + // proportionally, so their dividers end up off the extents they + // were given, and nothing else would ever correct that. Ask each + // nested imposed split to apply its extent again now that its + // container has settled. Runs at most once per retry, and + // retries are budgeted. + if abs((lastImposedOutcome ?? target) - target) <= 0.01 { + renudgeImposedDescendants(of: splitState) + } #if DEBUG dlog( "bonsplit.impose.retry split=\(String(splitState.id.uuidString.prefix(5)))" @@ -859,6 +953,17 @@ struct SplitContainerView: NSViewRepresentabl } } + private func renudgeImposedDescendants(of state: SplitState) { + for child in [state.first, state.second] { + guard case .split(let childState) = child else { continue } + if childState.imposedFirstExtent != nil { + childState.imposedEpoch &+= 1 + childState.applyImposedNow?() + } + renudgeImposedDescendants(of: childState) + } + } + func setPositionSafely(_ position: CGFloat, in splitView: NSSplitView, layout: Bool = true) { #if DEBUG // Wobble hunt: name every divider write while an imposition is @@ -948,8 +1053,6 @@ struct SplitContainerView: NSViewRepresentabl // `current != target` then re-layouts on every pass — a // main-thread spin. Re-apply only when the target changed or // something ELSE moved the divider off our last outcome. - let moved = abs(current - (lastImposedOutcome ?? .infinity)) > 0.01 - let retargeted = abs(target - (lastImposedTarget ?? .infinity)) > 0.01 // A fresh imposition call re-arms one apply attempt even for // an identical target: AppKit may have refused this exact // target earlier (transient pane minimums mid-churn), and @@ -957,21 +1060,32 @@ struct SplitContainerView: NSViewRepresentabl // would ever retry — panes would sit wedged at the refused // layout. Bounded by explicit calls, so it cannot spin. let renudged = lastImposedEpoch != splitState.imposedEpoch - // Never apply when the divider already sits at the target: - // a same-position setPosition still runs a layout pass, - // which re-applies surface sizes, which re-imposes — a - // sustained once-per-turn churn loop at full CPU. Renudge - // and retarget only bypass the refusal memo; distance from - // the target is what justifies touching AppKit. + // A new imposition always applies. Beyond that, when the + // divider is not where we left it, what to do depends on + // whether the split view itself was resized since we last + // applied. If it was, our stored extent is out of date and + // the host will send a new one computed for the new size — + // re-applying the old one here just moves the divider back + // and forth against AppKit on every update until it does. + // If the split view is the SAME size, no new extent is + // coming (the host only recomputes when something it can see + // changed), so a nudged divider would stay wrong forever — + // put it back; one apply settles it, since applying cannot + // resize the split view. And never apply when the divider is + // already at the target: a same-position setPosition still + // runs a layout pass, which re-applies surface sizes, which + // re-imposes — a once-per-turn churn loop at full CPU. + let moved = abs(current - (lastImposedOutcome ?? .infinity)) > 0.01 + let availUnchanged = abs(available - (lastImposedAvail ?? -1)) <= 0.01 if abs(current - target) <= 0.01 { - lastImposedTarget = target lastImposedEpoch = splitState.imposedEpoch lastImposedOutcome = current + lastImposedAvail = available imposedRetryBudget = 0 - } else if renudged || retargeted || moved { + } else if renudged || (moved && availUnchanged) { setPositionSafely(target, in: splitView, layout: true) - lastImposedTarget = target lastImposedEpoch = splitState.imposedEpoch + lastImposedAvail = available lastImposedOutcome = splitState.orientation == .horizontal ? splitView.arrangedSubviews[0].frame.width : splitView.arrangedSubviews[0].frame.height @@ -979,8 +1093,7 @@ struct SplitContainerView: NSViewRepresentabl dlog( "bonsplit.impose split=\(String(splitState.id.uuidString.prefix(5)))" + " target=\(Int(target)) outcome=\(Int(lastImposedOutcome ?? -1))" - + " avail=\(Int(available)) renudged=\(renudged ? 1 : 0)" - + " retargeted=\(retargeted ? 1 : 0) moved=\(moved ? 1 : 0)" + + " avail=\(Int(available))" ) #endif // A refused apply (AppKit clamped against constraints @@ -1231,12 +1344,22 @@ struct SplitContainerView: NSViewRepresentabl "divider.resizeIgnored split=\(splitState.id.uuidString.prefix(5)) eventType=\(eventType) leftDown=\(leftDown ? 1 : 0) isDragging=\(isDragging ? 1 : 0) normalized=\(String(format: "%.3f", normalizedPosition)) model=\(String(format: "%.3f", self.splitState.dividerPosition))" ) #endif - let statePosition = self.splitState.dividerPosition - // Re-assert synchronously. setPositionSafely sets isSyncingProgrammatically=true, - // so the recursive splitViewDidResizeSubviews call is caught by the guard above. - // Deferring to the next runloop turn would allow the transient frame to propagate - // through SwiftUI layout → ghostty terminal resize → reflow, causing content shifts. - self.syncPosition(statePosition, in: splitView) + // A split the user positions by fraction puts its divider + // back right here, synchronously (setPositionSafely sets + // isSyncingProgrammatically, so the recursive didResize is + // caught by the guard above; waiting a turn would let the + // in-between frame reach ghostty and reflow content). A + // split with an imposed extent must NOT do that: when its + // container resizes, the stored extent is out of date, and + // putting the divider back from inside the very layout + // pass that moved it starts another layout pass — that + // recursion is what pinned the main thread. The host sends + // a new extent for the new size; until it lands, a + // not-yet-right frame is fine. + if self.splitState.imposedFirstExtent == nil { + let statePosition = self.splitState.dividerPosition + self.syncPosition(statePosition, in: splitView) + } self.onGeometryChange?(false) return } diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index 5d90ae6a..3afe766a 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -119,6 +119,30 @@ public final class BonsplitController { self.configuration = configuration self.internalController = SplitViewController() internalController.publicController = self + internalController.onDividerDragSessionChange = { [weak self] active in + guard let self else { return } + if active { + self.delegate?.splitTabBarDividerDragDidBegin(self) + } else { + self.delegate?.splitTabBarDividerDragDidEnd(self) + } + } + } + + /// True while the user is dragging a divider anywhere in this tree. + /// Sessions bracket the divider's mouse-tracking lifecycle, so this is + /// authoritative — not inferred from resize callbacks. While true, + /// external sizing must not impose extents: the drag owns geometry. + public var isDividerDragActive: Bool { + internalController.activeDividerDragSessions > 0 + } + + /// Brackets a divider drag session from outside the built-in divider + /// tracking — for hosts with custom drag surfaces, and for tests that + /// need the session state without synthesizing mouse events. Balanced + /// begin/end pairs are the caller's responsibility. + public func noteDividerDragSession(_ active: Bool) { + internalController.noteDividerDragSession(active) } // MARK: - Tab Operations diff --git a/Sources/Bonsplit/Public/BonsplitDelegate.swift b/Sources/Bonsplit/Public/BonsplitDelegate.swift index 1444a418..d76b3668 100644 --- a/Sources/Bonsplit/Public/BonsplitDelegate.swift +++ b/Sources/Bonsplit/Public/BonsplitDelegate.swift @@ -76,6 +76,17 @@ public protocol BonsplitDelegate: AnyObject { /// Called to check if notifications should be sent during divider drag (opt-in for real-time sync) func splitTabBar(_ controller: BonsplitController, shouldNotifyDuringDrag: Bool) -> Bool + + /// A divider drag session started: the mouse went down on a divider and + /// AppKit's tracking loop is running. Until the matching end, the drag + /// owns divider geometry — hosts that impose extents should defer. + func splitTabBarDividerDragDidBegin(_ controller: BonsplitController) + + /// The divider drag session ended (mouse released). Delivered from the + /// tracking lifecycle itself, so it fires even when no resize callback + /// coincides with the release; the final geometry has already been + /// reported through `didChangeGeometry`. + func splitTabBarDividerDragDidEnd(_ controller: BonsplitController) } // MARK: - Default Implementations (all methods optional) @@ -99,4 +110,6 @@ public extension BonsplitDelegate { func splitTabBar(_ controller: BonsplitController, didRequestTabMoveToDestination destinationId: String, for tab: Tab, inPane pane: PaneID) {} func splitTabBar(_ controller: BonsplitController, didChangeGeometry snapshot: LayoutSnapshot) {} func splitTabBar(_ controller: BonsplitController, shouldNotifyDuringDrag: Bool) -> Bool { false } + func splitTabBarDividerDragDidBegin(_ controller: BonsplitController) {} + func splitTabBarDividerDragDidEnd(_ controller: BonsplitController) {} } From 4858deec8867db1862029fbf26b71e9a14e873a5 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 03:34:00 -0700 Subject: [PATCH 02/10] Notify drag sessions only on the zero crossing; widen the session to any divider mouseDown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With overlapping sessions (a host-bracketed custom drag alongside the built-in tracking), ending one must not announce the drag over while the other still owns the divider. And the session hit test cannot faithfully reconstruct the effective divider rect AppKit tracks (our expansion unioned with AppKit's own proposal), so any mouseDown that reaches the split view — content clicks never do — brackets a session; a session around a non-drag click is a harmless no-op pair. setImposedFirstExtent now documents that a stored extent is not re-asserted across container resizes: the host imposes fresh values for new sizes. --- .../Controllers/SplitViewController.swift | 10 ++++- .../Internal/Views/SplitContainerView.swift | 39 ++++++------------- .../Bonsplit/Public/BonsplitController.swift | 9 +++++ 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift index 3e4262c6..4c14afc5 100644 --- a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift +++ b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift @@ -78,8 +78,16 @@ final class SplitViewController { @ObservationIgnored var onDividerDragSessionChange: ((Bool) -> Void)? func noteDividerDragSession(_ active: Bool) { + // Notify only when the count crosses zero: with overlapping sessions + // (a host-bracketed custom drag alongside the built-in tracking), + // ending one must not announce "drag over" while the other still + // owns the divider — a host would resume imposing under the pointer. + let wasActive = activeDividerDragSessions > 0 activeDividerDragSessions = max(0, activeDividerDragSessions + (active ? 1 : -1)) - onDividerDragSessionChange?(active) + let isActive = activeDividerDragSessions > 0 + if wasActive != isActive { + onDividerDragSessionChange?(isActive) + } } init(rootNode: SplitNode? = nil) { diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index 2e7ef88d..994d6768 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -106,36 +106,21 @@ private class ThemedSplitView: NSSplitView, BonsplitManagedSplitView { /// drag-pause-release, where no resize coincides with the mouseUp). var onDividerDragSession: ((Bool) -> Void)? - private func dividerSessionHitRect() -> NSRect? { - // No minimum-size guards here, deliberately: a pane parked at its - // 1pt minimum is exactly the pane a user grabs the divider to - // restore, and AppKit still tracks that drag (the delegate's - // effectiveRect has no such guard either). A session that fails to - // arm there would let sizing passes impose over the live drag. - guard arrangedSubviews.count >= 2 else { return nil } - let a = arrangedSubviews[0].frame - let thickness = dividerThickness - let rect: NSRect - if isVertical { - rect = NSRect(x: max(0, a.maxX), y: 0, width: thickness, height: bounds.height) - } else { - rect = NSRect(x: 0, y: max(0, a.maxY), width: bounds.width, height: thickness) - } - return rect.insetBy(dx: -resolvedDividerHitExpansion, dy: -resolvedDividerHitExpansion) - } - override func mouseDown(with event: NSEvent) { - let location = convert(event.locationInWindow, from: nil) - // Max-edge INCLUSIVE, matching the delegate's dividerHitRectContains: - // AppKit can report a point exactly on the divider boundary, and an - // exclusive contains() would miss the session while NSSplitView still - // tracks the drag. - let inDivider = dividerSessionHitRect().map { rect in - location.x >= rect.minX && location.x <= rect.maxX - && location.y >= rect.minY && location.y <= rect.maxY - } == true + // Any mouseDown that reaches the split view itself is a divider + // interaction: arranged subviews cover all pane content, so content + // clicks never route here, and reconstructing AppKit's exact + // effective divider rect (drawn rect grown by the delegate's + // expansion, UNIONED with AppKit's own proposal) cannot be done + // faithfully from here — a rect test narrower than AppKit's would + // let AppKit track a drag with no session, and sizing would impose + // under the pointer. A session around a click that AppKit does not + // turn into a drag is harmless: begin and end fire back to back and + // the host's drag-end sync finds nothing changed. + let inDivider = arrangedSubviews.count >= 2 #if DEBUG if inDivider { + let location = convert(event.locationInWindow, from: nil) dlog("divider.session.mouseDown loc=\(Int(location.x)),\(Int(location.y))") } #endif diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index 3afe766a..ebbf8845 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -1048,6 +1048,15 @@ public final class BonsplitController { /// express the required pixel size losslessly. Repeating the same extent /// is idempotent; after an external constraint change, use /// `retryImposedFirstExtent(forSplit:)` to request one fresh bounded apply. + /// + /// The extent is applied when set (and re-applied if the divider drifts + /// while the split view's own size is unchanged), but it is NOT + /// re-asserted after the split view resizes: a stored extent computed + /// for the old size is stale at the new one, and re-applying it from + /// inside the resize's layout pass fights AppKit recursively. A host + /// that imposes extents is expected to impose fresh values when the + /// container it derived them from changes; until it does, the divider + /// rides AppKit's proportional resize. public func setImposedFirstExtent( _ extent: CGFloat?, forSplit splitId: UUID, fromExternal: Bool = false ) -> Bool { From 2b2b795dca0212894cfd6f806bd340db57a73b5f Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 08:57:40 -0700 Subject: [PATCH 03/10] Refuse imposed-extent retries while a drag session owns the divider retryImposedFirstExtent re-asserts a stored extent on host recovery triggers (rebuilds, tab re-shows, reconnects). Fired mid-drag it would move the divider under the pointer, so it now returns false while any drag session is active for the tree; the host's drag-end sync imposes fresh values once the session closes. Also reconciles the drag-session line with the imposed-first-extent rework it now sits on: renudgeImposedDescendants uses the renamed syncDividerNow hook, and reapplyDividerForThicknessChange records the epoch/outcome/avail memo instead of the removed lastImposedTarget. --- Sources/Bonsplit/Internal/Views/SplitContainerView.swift | 4 ++-- Sources/Bonsplit/Public/BonsplitController.swift | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index 994d6768..6727e4c1 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -943,7 +943,7 @@ struct SplitContainerView: NSViewRepresentabl guard case .split(let childState) = child else { continue } if childState.imposedFirstExtent != nil { childState.imposedEpoch &+= 1 - childState.applyImposedNow?() + childState.syncDividerNow?() } renudgeImposedDescendants(of: childState) } @@ -991,7 +991,7 @@ struct SplitContainerView: NSViewRepresentabl if let imposed = splitState.imposedFirstExtent { let target = clampedDividerPosition(imposed, in: splitView) setPositionSafely(target, in: splitView, layout: true) - lastImposedTarget = target + lastImposedAvail = available lastImposedEpoch = splitState.imposedEpoch lastImposedOutcome = splitState.orientation == .horizontal ? splitView.arrangedSubviews[0].frame.width diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index ebbf8845..bb55e72b 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -1085,6 +1085,11 @@ public final class BonsplitController { /// `setImposedFirstExtent(_:forSplit:fromExternal:)`, whose identical /// updates are intentionally idempotent. public func retryImposedFirstExtent(forSplit splitId: UUID) -> Bool { + // A live drag session owns the divider: re-asserting an imposed + // extent now would yank it out from under the pointer mid-gesture. + // Refuse instead of deferring — the host's drag-end sync imposes + // fresh values (or retries) once the session closes. + guard internalController.activeDividerDragSessions == 0 else { return false } guard let split = internalController.findSplit(splitId), split.imposedFirstExtent != nil else { return false } From 4f91c547bcb72453ab7e9eac2141f77755c4dc9e Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 09:35:31 -0700 Subject: [PATCH 04/10] Deliver the final geometry notification before drag-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On session end the controller notified the delegate first, so drag-end arrived before the coordinator's final didChangeGeometry — inverting the documented contract that the settled geometry has already been reported when drag-end runs. A host that buffers geometry callbacks and commits at drag-end would commit pre-drag state. The coordinator now leads on both edges of the session; hosts that suppress geometry callbacks while the session is live lose nothing, since the model is final at release and drag-end is the signal to read it. --- .../Internal/Views/SplitContainerView.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index 6727e4c1..06610211 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -283,17 +283,20 @@ struct SplitContainerView: NSViewRepresentabl context.coordinator.splitView = splitView let internalController = controller splitView.onDividerDragSession = { [weak coordinator = context.coordinator, weak internalController] active in - // Order matters at session END: the counter must drop to zero - // BEFORE the coordinator's final geometry notification, or the - // delegate still reads the drag as live and skips the drag-end - // sync. At BEGIN the coordinator arms first so isDragging is set - // before any delegate reacts to the session. + // The coordinator leads on both edges. At begin it arms isDragging + // before any delegate reacts to the session. At end it delivers the + // final geometry notification before the counter drops and the + // delegate hears drag-end: the delegate contract promises the + // settled geometry has already been reported when drag-end runs. + // Hosts that suppress geometry callbacks while the session is live + // lose nothing — the model is final by then, and drag-end is the + // signal to read it. if active { coordinator?.dividerDragSessionChanged(true) internalController?.noteDividerDragSession(true) } else { - internalController?.noteDividerDragSession(false) coordinator?.dividerDragSessionChanged(false) + internalController?.noteDividerDragSession(false) } } From a7c9a81db426e41d128146d56a810134eedc2357 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 09:54:34 -0700 Subject: [PATCH 05/10] Re-arm one apply on every explicit imposition, even for a repeated extent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A container resize proportionally rescales dividers, so an imposed split can drift off its extent. The host re-imposes after the resize, but the fresh plan often computes the same extent as before — per-pane ideals do not depend on the container — and setImposedFirstExtent deduped that call by value, bumping no epoch. syncPosition then saw an unchanged epoch with a changed container and, per the one-writer rule, refused to re-apply: the divider stayed wedged at the drifted position until some input happened to change the number. Live signature: a settled pane one row short of its tmux assignment, persisting through reconfirm. Every explicit non-nil call now bumps the epoch, matching syncPosition's documented 'bounded by explicit calls' contract. A repeat while the divider is already at target is memo-only (no layout pass), so the re-arm cannot churn. Regression: testReimposingSameExtentAfterContainerResizeRetargetsExactly imposes 120 in 400pt, shrinks to 300 (drift to 90), re-imposes the same 120 — red without the fix, green with it. --- .../Bonsplit/Public/BonsplitController.swift | 20 +++- Tests/BonsplitTests/BonsplitTests.swift | 93 +++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index bb55e72b..7886c7e3 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -1045,9 +1045,14 @@ public final class BonsplitController { /// for readers. A user divider drag clears the imposition and returns /// the split to fraction semantics. Use this when pane contents are /// grid-quantized (terminal cells) and a normalized fraction cannot - /// express the required pixel size losslessly. Repeating the same extent - /// is idempotent; after an external constraint change, use - /// `retryImposedFirstExtent(forSplit:)` to request one fresh bounded apply. + /// express the required pixel size losslessly. Every explicit call + /// re-arms one bounded apply, including a call that repeats the stored + /// extent: hosts re-impose after their container resizes, and the fresh + /// plan often computes the SAME extent (per-pane ideals are container- + /// independent) while the resize moved the divider off it — deduping the + /// call by value would leave the divider wedged at the drifted position. + /// Repeating the extent stays cheap when the divider is already at the + /// target (the coordinator refreshes its memos without a layout pass). /// /// The extent is applied when set (and re-applied if the divider drifts /// while the split view's own size is unchanged), but it is NOT @@ -1068,6 +1073,15 @@ public final class BonsplitController { if split.imposedFirstExtent != normalizedExtent { split.imposedFirstExtent = normalizedExtent split.imposedEpoch &+= 1 + } else if normalizedExtent != nil { + // Same extent, explicit call: re-arm one apply anyway. The + // coordinator's renudge path is memo-only when the divider is + // already at the target, so this cannot churn layout; when the + // divider drifted (a container resize rescaled it while the + // re-planned extent came out identical), this is the only thing + // that puts it back — see `syncPosition`'s "bounded by explicit + // calls" contract. + split.imposedEpoch &+= 1 } split.syncDividerNow?() if fromExternal { diff --git a/Tests/BonsplitTests/BonsplitTests.swift b/Tests/BonsplitTests/BonsplitTests.swift index 60a211fb..a4ad54c0 100644 --- a/Tests/BonsplitTests/BonsplitTests.swift +++ b/Tests/BonsplitTests/BonsplitTests.swift @@ -3506,6 +3506,99 @@ final class BonsplitTests: XCTestCase { ) } + /// A host that imposes exact extents re-imposes after its container + /// resizes, and the fresh plan often computes the SAME number: per-pane + /// ideals are container-independent (a row of N terminal cells is the + /// same points in any container). Meanwhile AppKit's proportional resize + /// has moved the divider off the imposed extent. The coordinator's + /// renudge path documents that every explicit imposition call re-arms + /// one apply ("bounded by explicit calls"), so the identical re-imposition + /// must put the divider back — deduping it by value leaves the divider + /// wedged at the proportional position until some input happens to + /// change the number. + @MainActor + func testReimposingSameExtentAfterContainerResizeRetargetsExactly() throws { + var configuration = BonsplitConfiguration() + configuration.appearance.minimumPaneWidth = 1 + configuration.appearance.minimumPaneHeight = 1 + configuration.appearance.enableAnimations = false + configuration.dividerPositionRange = 0...1 + let controller = BonsplitController(configuration: configuration) + _ = controller.createTab(title: "Left") + let sourcePane = try XCTUnwrap(controller.focusedPaneId) + XCTAssertNotNil(controller.splitPane( + sourcePane, + orientation: .horizontal, + initialDividerPosition: 0.5 + )) + guard case .split(let split) = controller.treeSnapshot() else { + XCTFail("Expected split root") + return + } + let splitId = try XCTUnwrap(UUID(uuidString: split.id)) + + let hostingView = NSHostingView( + rootView: BonsplitView(controller: controller) { _, _ in + Color.clear + } emptyPane: { _ in + Color.clear + } + ) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + let contentView = try XCTUnwrap(window.contentView) + hostingView.frame = contentView.bounds + hostingView.autoresizingMask = [.width, .height] + contentView.addSubview(hostingView) + window.makeKeyAndOrderFront(nil) + contentView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.05)) + contentView.layoutSubtreeIfNeeded() + let splitView = try XCTUnwrap(firstDescendant(ofType: NSSplitView.self, in: hostingView)) + XCTAssertEqual(splitView.arrangedSubviews.count, 2) + + func settleImposed(_ extent: CGFloat) -> CGFloat { + _ = controller.setImposedFirstExtent(extent, forSplit: splitId, fromExternal: true) + for _ in 0..<12 { + splitView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + contentView.layoutSubtreeIfNeeded() + if abs(splitView.arrangedSubviews[0].frame.width - extent) <= 1 { break } + } + return splitView.arrangedSubviews[0].frame.width + } + + let imposed = CGFloat(120) + XCTAssertEqual(settleImposed(imposed), imposed, accuracy: 1.5) + + // The container shrinks; AppKit rescales the split proportionally and + // the divider drifts off the imposed extent (one-writer: nothing puts + // it back until the host re-imposes). + window.setContentSize(NSSize(width: 300, height: 300)) + for _ in 0..<12 { + contentView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + } + let drifted = splitView.arrangedSubviews[0].frame.width + XCTAssertLessThan( + drifted, + imposed - 4, + "expected the proportional resize to move the divider off the imposed extent" + ) + + XCTAssertEqual( + settleImposed(imposed), + imposed, + accuracy: 1.5, + "identical re-imposition after a container resize was deduped away; the divider stayed at the drifted position" + ) + } + @MainActor func testTranslucentSplitWrappersStayClear() { let appearance = BonsplitConfiguration.Appearance( From d0a6c15716cb1def12443fa16c5e74b4d8c43b27 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 12:03:25 -0700 Subject: [PATCH 06/10] Refuse every imposed apply while a drag session owns the divider The public retryImposedFirstExtent already refused during a drag session, but the internal apply paths did not: a mid-drag setImposedFirstExtent (or a drift renudge, a descendant renudge, or a deferred syncDividerNow apply) still ran its main-queue apply during AppKit's tracking loop and yanked the divider out from under the pointer, re-marking the split imposed against the drag's own clear. Gate the apply at its funnel instead of per API. Every imposed apply runs through the coordinator's syncPosition imposed branch or its deferred retry, so those two points now consult the tree's live drag session count and refuse outright: no divider write, no memo or epoch bookkeeping, no retry budget consumed. The pending extent stays armed. When the session count crosses back to zero, the internal controller gives every still-imposed split one deferred apply. There is no epoch bump, so a split already at its target only refreshes its memos, and an interrupted retry chain resumes from the same sync with the budget it had left. Drift renudges with no session active keep working as before. --- .../Controllers/SplitViewController.swift | 11 +++ .../Internal/Views/SplitContainerView.swift | 34 ++++++++ Tests/BonsplitTests/BonsplitTests.swift | 87 +++++++++++++++++++ 3 files changed, 132 insertions(+) diff --git a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift index 4c14afc5..aebfb066 100644 --- a/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift +++ b/Sources/Bonsplit/Internal/Controllers/SplitViewController.swift @@ -87,6 +87,17 @@ final class SplitViewController { let isActive = activeDividerDragSessions > 0 if wasActive != isActive { onDividerDragSessionChange?(isActive) + if !isActive { + // Imposed applies refuse while a session is live and stay + // armed. Give every still-imposed split one deferred apply + // now that the drag released the divider — no epoch bump, so + // a split already at its target just refreshes its memos. + // The split that was dragged cleared its imposition when the + // gesture took ownership, so it skips itself here. + for split in allSplits where split.imposedFirstExtent != nil { + split.syncDividerNow?() + } + } } } diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index 06610211..aa2e5306 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -282,6 +282,9 @@ struct SplitContainerView: NSViewRepresentabl context.coordinator.splitView = splitView let internalController = controller + context.coordinator.isTreeDragSessionActive = { [weak internalController] in + (internalController?.activeDividerDragSessions ?? 0) > 0 + } splitView.onDividerDragSession = { [weak coordinator = context.coordinator, weak internalController] active in // The coordinator leads on both edges. At begin it arms isDragging // before any delegate reacts to the session. At end it delivers the @@ -506,6 +509,12 @@ struct SplitContainerView: NSViewRepresentabl (splitView as? ThemedSplitView)?.customDividerHitExpansion = appearance.dividerHitExpansion (splitView as? ThemedSplitView)?.stampedInternalController = controller (splitView as? ThemedSplitView)?.stampedSplitId = splitState.id + // Re-install alongside the identity stamps above so a reused + // coordinator keeps answering for the tree it currently renders. + let internalController = controller + context.coordinator.isTreeDragSessionActive = { [weak internalController] in + (internalController?.activeDividerDragSessions ?? 0) > 0 + } // Update orientation if changed splitView.isVertical = splitState.orientation == .horizontal @@ -698,6 +707,11 @@ struct SplitContainerView: NSViewRepresentabl /// Retry a few turns so entry animations are not dropped on first layout. var initialDividerApplyAttempts = 0 var onGeometryChange: ((_ isDragging: Bool) -> Void)? + /// Answers whether a divider drag session is live anywhere in this + /// tree. Consulted before every imposed apply: mid-drag the user owns + /// the divider, so an apply refuses and stays armed instead of moving + /// it under the pointer. Installed from makeNSView/updateNSView. + var isTreeDragSessionActive: (() -> Bool)? /// Track last applied position to detect external changes var lastAppliedPosition: CGFloat = 0.5 /// What the last imposed apply actually produced, and how big the @@ -903,6 +917,9 @@ struct SplitContainerView: NSViewRepresentabl let imposed = splitState.imposedFirstExtent, splitView.arrangedSubviews.count >= 2 else { return } + // Refuse mid-drag without consuming budget: the chain resumes + // from syncPosition when the session-end renudge lands. + if isTreeDragSessionActive?() == true { return } imposedRetryBudget -= 1 let target = clampedDividerPosition(imposed, in: splitView) let current = splitState.orientation == .horizontal @@ -1027,6 +1044,17 @@ struct SplitContainerView: NSViewRepresentabl // exact points and mirrors the resulting fraction back into the // model for readers. if let imposed = splitState.imposedFirstExtent { + // While a drag session owns a divider anywhere in this tree, + // an imposed apply refuses outright: no divider write, no + // memo or epoch bookkeeping, no retry-budget consumption. + // Every imposed-apply trigger funnels through here (fresh + // impositions via syncDividerNow, drift renudges, descendant + // renudges), so this is the single gate that keeps a + // main-queue apply from yanking the divider out from under + // the pointer mid-gesture. The pending extent stays armed; + // the internal controller's session-end renudge re-runs this + // sync once the user's hand is off the divider. + if isTreeDragSessionActive?() == true { return } guard splitView.arrangedSubviews.count >= 2 else { return } let available = splitAvailableSize(in: splitView) guard available > 0 else { return } @@ -1098,6 +1126,12 @@ struct SplitContainerView: NSViewRepresentabl } else { imposedRetryBudget = 0 } + } else if imposedRetryBudget > 0 { + // A retry chain the drag-session gate interrupted parked + // here with budget left; the session-end renudge lands in + // this sync, so pick the chain back up. + imposedRetrySplitView = splitView + scheduleImposedRetry() } mirrorImposedOutcome(lastImposedOutcome ?? current, in: splitView) return diff --git a/Tests/BonsplitTests/BonsplitTests.swift b/Tests/BonsplitTests/BonsplitTests.swift index a4ad54c0..610980b3 100644 --- a/Tests/BonsplitTests/BonsplitTests.swift +++ b/Tests/BonsplitTests/BonsplitTests.swift @@ -3599,6 +3599,93 @@ final class BonsplitTests: XCTestCase { ) } + /// While a divider drag session is live, the user owns the divider: an + /// imposed extent arriving mid-session must not move it. The imposition + /// stays armed instead, and the session's end applies it with no fresh + /// impose call from the host. + @MainActor + func testImposeDuringDragSessionDefersUntilSessionEnds() throws { + var configuration = BonsplitConfiguration() + configuration.appearance.minimumPaneWidth = 1 + configuration.appearance.minimumPaneHeight = 1 + configuration.appearance.enableAnimations = false + configuration.dividerPositionRange = 0...1 + let controller = BonsplitController(configuration: configuration) + _ = controller.createTab(title: "Left") + let sourcePane = try XCTUnwrap(controller.focusedPaneId) + XCTAssertNotNil(controller.splitPane( + sourcePane, + orientation: .horizontal, + initialDividerPosition: 0.5 + )) + guard case .split(let split) = controller.treeSnapshot() else { + XCTFail("Expected split root") + return + } + let splitId = try XCTUnwrap(UUID(uuidString: split.id)) + + let hostingView = NSHostingView( + rootView: BonsplitView(controller: controller) { _, _ in + Color.clear + } emptyPane: { _ in + Color.clear + } + ) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + let contentView = try XCTUnwrap(window.contentView) + hostingView.frame = contentView.bounds + hostingView.autoresizingMask = [.width, .height] + contentView.addSubview(hostingView) + window.makeKeyAndOrderFront(nil) + contentView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.05)) + contentView.layoutSubtreeIfNeeded() + let splitView = try XCTUnwrap(firstDescendant(ofType: NSSplitView.self, in: hostingView)) + XCTAssertEqual(splitView.arrangedSubviews.count, 2) + + let widthBeforeImpose = splitView.arrangedSubviews[0].frame.width + let imposed = CGFloat(120) + XCTAssertGreaterThan( + abs(widthBeforeImpose - imposed), + 20, + "the imposed extent must differ from the current divider for this test to mean anything" + ) + + controller.noteDividerDragSession(true) + XCTAssertTrue(controller.setImposedFirstExtent(imposed, forSplit: splitId)) + for _ in 0..<6 { + splitView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + contentView.layoutSubtreeIfNeeded() + } + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + widthBeforeImpose, + accuracy: 1.5, + "an imposed extent must not move the divider while a drag session is live" + ) + + controller.noteDividerDragSession(false) + for _ in 0..<12 { + splitView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + contentView.layoutSubtreeIfNeeded() + if abs(splitView.arrangedSubviews[0].frame.width - imposed) <= 1 { break } + } + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + imposed, + accuracy: 1.5, + "the armed extent must apply once the session closes, with no fresh impose call" + ) + } + @MainActor func testTranslucentSplitWrappersStayClear() { let appearance = BonsplitConfiguration.Appearance( From 3273b80e987bf46e6b81f2963763b82dd1e9a6b7 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 12:04:00 -0700 Subject: [PATCH 07/10] Force the drag-end geometry notification past the external-update window The guaranteed final notification funneled through notifyGeometryChange, whose first line drops everything within ~50ms of a fromExternal update. A quick flick released inside that window lost the one notification the delegate contract promises has already been delivered when dragDidEnd runs. Deliver it from the session counter's zero crossing in the controller instead, with an explicit force that bypasses only the suppression window and only for this path; every other caller keeps the gate. The zero crossing also covers host-bracketed sessions via noteDividerDragSession, which never ran the coordinator's drag-end path at all, and it still fires before splitTabBarDividerDragDidEnd, so the ordering the contract promises is unchanged. The coordinator's session-end callback now only clears its drag flag. --- .../Internal/Views/SplitContainerView.swift | 34 +++++----- .../Bonsplit/Public/BonsplitController.swift | 19 +++++- .../Bonsplit/Public/BonsplitDelegate.swift | 4 +- Tests/BonsplitTests/BonsplitTests.swift | 65 +++++++++++++++++++ 4 files changed, 100 insertions(+), 22 deletions(-) diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index aa2e5306..3f164d5f 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -286,13 +286,14 @@ struct SplitContainerView: NSViewRepresentabl (internalController?.activeDividerDragSessions ?? 0) > 0 } splitView.onDividerDragSession = { [weak coordinator = context.coordinator, weak internalController] active in - // The coordinator leads on both edges. At begin it arms isDragging - // before any delegate reacts to the session. At end it delivers the - // final geometry notification before the counter drops and the - // delegate hears drag-end: the delegate contract promises the - // settled geometry has already been reported when drag-end runs. - // Hosts that suppress geometry callbacks while the session is live - // lose nothing — the model is final by then, and drag-end is the + // The coordinator arms isDragging at begin, before any delegate + // reacts to the session. At end the counter's zero crossing + // delivers the final geometry notification — forced past the + // external-update suppression window — before the delegate hears + // drag-end: the delegate contract promises the settled geometry + // has already been reported when drag-end runs. Hosts that + // suppress geometry callbacks while the session is live lose + // nothing — the model is final by then, and drag-end is the // signal to read it. if active { coordinator?.dividerDragSessionChanged(true) @@ -790,21 +791,18 @@ struct SplitContainerView: NSViewRepresentabl /// Deterministic drag session, bracketed by `ThemedSplitView.mouseDown` /// around AppKit's divider tracking loop. Begin arms the drag before - /// any resize callback needs to infer it; end always fires at release - /// and delivers the drag-end geometry notification — whether or not a - /// final resize callback coincided with the mouseUp. The model itself - /// is maintained by the resize callbacks during the drag; end only has - /// to announce that the user's hand is off the divider. + /// any resize callback needs to infer it; end always fires at release. + /// The model itself is maintained by the resize callbacks during the + /// drag; the guaranteed drag-end geometry notification is delivered by + /// the internal controller's zero crossing right after this — from + /// there it bypasses the external-update suppression window, which + /// would otherwise swallow a release landing within ~50ms of a + /// fromExternal call. func dividerDragSessionChanged(_ active: Bool) { #if DEBUG dlog("divider.session split=\(String(splitState.id.uuidString.prefix(5))) \(active ? "begin" : "end")") #endif - if active { - isDragging = true - return - } - isDragging = false - onGeometryChange?(false) + isDragging = active } private func splitTotalSize(in splitView: NSSplitView) -> CGFloat { diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index 7886c7e3..7c076910 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -124,6 +124,14 @@ public final class BonsplitController { if active { self.delegate?.splitTabBarDividerDragDidBegin(self) } else { + // Drag-end promises the settled geometry has already been + // reported when dragDidEnd runs. Deliver it here, on the + // session's zero crossing, and force it past the external- + // update suppression window: a quick flick can release + // within that window's ~50ms of a fromExternal call, and + // the gate would silently swallow the one notification the + // contract guarantees. + self.notifyGeometryChange(force: true) self.delegate?.splitTabBarDividerDragDidEnd(self) } } @@ -1118,9 +1126,14 @@ public final class BonsplitController { } /// Notify geometry change to delegate (internal use) - /// - Parameter isDragging: Whether the change is due to active divider dragging - internal func notifyGeometryChange(isDragging: Bool = false) { - guard !internalController.isExternalUpdateInProgress else { return } + /// - Parameters: + /// - isDragging: Whether the change is due to active divider dragging + /// - force: Deliver even inside the external-update suppression window. + /// Only the drag-end path passes true: its notification is the one + /// the delegate contract guarantees, so the anti-echo gate must not + /// swallow it. + internal func notifyGeometryChange(isDragging: Bool = false, force: Bool = false) { + guard force || !internalController.isExternalUpdateInProgress else { return } // If dragging, check if delegate wants notifications during drag if isDragging { diff --git a/Sources/Bonsplit/Public/BonsplitDelegate.swift b/Sources/Bonsplit/Public/BonsplitDelegate.swift index d76b3668..1e127590 100644 --- a/Sources/Bonsplit/Public/BonsplitDelegate.swift +++ b/Sources/Bonsplit/Public/BonsplitDelegate.swift @@ -85,7 +85,9 @@ public protocol BonsplitDelegate: AnyObject { /// The divider drag session ended (mouse released). Delivered from the /// tracking lifecycle itself, so it fires even when no resize callback /// coincides with the release; the final geometry has already been - /// reported through `didChangeGeometry`. + /// reported through `didChangeGeometry`. That report is delivered even + /// inside the brief suppression window a `fromExternal` update opens, so + /// a release right after an external echo cannot lose it. func splitTabBarDividerDragDidEnd(_ controller: BonsplitController) } diff --git a/Tests/BonsplitTests/BonsplitTests.swift b/Tests/BonsplitTests/BonsplitTests.swift index 610980b3..b8cd80a6 100644 --- a/Tests/BonsplitTests/BonsplitTests.swift +++ b/Tests/BonsplitTests/BonsplitTests.swift @@ -146,6 +146,28 @@ final class BonsplitTests: XCTestCase { } } + private final class DividerDragEventRecorder: BonsplitDelegate { + enum Event: Equatable { + case dragBegan + case dragEnded + case geometryChanged + } + + var events: [Event] = [] + + func splitTabBarDividerDragDidBegin(_ controller: BonsplitController) { + events.append(.dragBegan) + } + + func splitTabBarDividerDragDidEnd(_ controller: BonsplitController) { + events.append(.dragEnded) + } + + func splitTabBar(_ controller: BonsplitController, didChangeGeometry snapshot: LayoutSnapshot) { + events.append(.geometryChanged) + } + } + private final class ObservationInvalidationFlag: @unchecked Sendable { var didInvalidate = false } @@ -3686,6 +3708,49 @@ final class BonsplitTests: XCTestCase { ) } + /// Drag-end promises the settled geometry has already been reported when + /// `splitTabBarDividerDragDidEnd` runs. A `fromExternal` update opens a + /// short suppression window for outgoing geometry notifications; a + /// session that ends inside that window (a quick flick released right + /// after the host echoed a position) must still get its final + /// `didChangeGeometry`, and get it before drag-end. + @MainActor + func testDragEndGeometryBypassesExternalUpdateSuppression() throws { + let controller = BonsplitController(configuration: BonsplitConfiguration( + appearance: .init(enableAnimations: false) + )) + let recorder = DividerDragEventRecorder() + controller.delegate = recorder + _ = controller.createTab(title: "Base") + let sourcePane = try XCTUnwrap(controller.focusedPaneId) + XCTAssertNotNil(controller.splitPane( + sourcePane, + orientation: .horizontal, + initialDividerPosition: 0.5 + )) + guard case .split(let split) = controller.treeSnapshot() else { + XCTFail("Expected split root") + return + } + let splitId = try XCTUnwrap(UUID(uuidString: split.id)) + + recorder.events.removeAll() + controller.noteDividerDragSession(true) + XCTAssertEqual(recorder.events, [.dragBegan]) + + // The external echo lands, then the session ends immediately — well + // inside the suppression window the external update opened. + XCTAssertTrue(controller.setDividerPosition(0.45, forSplit: splitId, fromExternal: true)) + recorder.events.removeAll() + controller.noteDividerDragSession(false) + + XCTAssertEqual( + recorder.events, + [.geometryChanged, .dragEnded], + "session end must deliver the final geometry, ahead of drag-end, even while an external update suppresses notifications" + ) + } + @MainActor func testTranslucentSplitWrappersStayClear() { let appearance = BonsplitConfiguration.Appearance( From 6568da9190009e5a6b74c40d67c1f0911a78ec70 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 16:01:50 -0700 Subject: [PATCH 08/10] Re-arm one bounded apply when a container resize parks an imposed divider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A window resize rescales an imposed split proportionally, moving the divider off its extent. Since the re-apply condition became `renudged || (moved && availUnchanged)`, that drift was left for the host to fix: the theory was that a resized container always gets a fresh imposition. But a host whose per-pane ideals are container-independent computes the SAME extent for any container size and only re-imposes when its own inputs change. If the epoch-bumped applies from the last imposition all expired against not-yet-final bounds (retry budget spent while a concurrent window resize was still committing), the park was permanent — a deterministic fuzz repro held the divider at the proportional position (plan 1199pt vs view 984pt) for 50+ seconds until an unrelated input. Give the parked divider one deferred apply of its own instead of waiting for the host. Both places that observe the size change — the didResize callback and syncPosition's imposed branch — record the new available size and arm a single retry against it. Recording the size immediately bounds this to one re-arm per size change, the apply runs a runloop turn later so it never fights AppKit from inside the resize's own layout pass, and applying cannot resize the split view, so it cannot re-trigger itself. A mid-drag retry still refuses without consuming its budget; the session-end renudge resumes the chain. The contract, now stated in the doc comments: an apply never terminates off-target without re-arming on the next size change. The new regression test imposes an extent, resizes the window with no further imposition and no drag session, and asserts the first pane returns to the extent; it fails without the fix (divider stays at 192pt instead of 120pt). The existing same-extent re-imposition test asserted the drift persisted until the host's call — updated to the new contract. --- .../Internal/Views/SplitContainerView.swift | 67 +++++++--- .../Bonsplit/Public/BonsplitController.swift | 24 ++-- Tests/BonsplitTests/BonsplitTests.swift | 116 +++++++++++++++--- 3 files changed, 161 insertions(+), 46 deletions(-) diff --git a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift index 3f164d5f..68778554 100644 --- a/Sources/Bonsplit/Internal/Views/SplitContainerView.swift +++ b/Sources/Bonsplit/Internal/Views/SplitContainerView.swift @@ -717,8 +717,8 @@ struct SplitContainerView: NSViewRepresentabl var lastAppliedPosition: CGFloat = 0.5 /// What the last imposed apply actually produced, and how big the /// split view was at the time. syncPosition compares against both to - /// decide whether a moved divider should be put back or left for the - /// host to re-impose at the new size. + /// decide whether a moved divider is put back synchronously (same + /// size) or re-armed for one deferred apply at the settled size. var lastImposedOutcome: CGFloat? var lastImposedAvail: CGFloat? var lastImposedEpoch: Int? @@ -1077,17 +1077,22 @@ struct SplitContainerView: NSViewRepresentabl // A new imposition always applies. Beyond that, when the // divider is not where we left it, what to do depends on // whether the split view itself was resized since we last - // applied. If it was, our stored extent is out of date and - // the host will send a new one computed for the new size — - // re-applying the old one here just moves the divider back - // and forth against AppKit on every update until it does. - // If the split view is the SAME size, no new extent is - // coming (the host only recomputes when something it can see - // changed), so a nudged divider would stay wrong forever — - // put it back; one apply settles it, since applying cannot - // resize the split view. And never apply when the divider is - // already at the target: a same-position setPosition still - // runs a layout pass, which re-applies surface sizes, which + // applied. If the split view is the SAME size, no new extent + // is coming (the host only recomputes when something it can + // see changed), so a nudged divider would stay wrong forever + // — put it back synchronously; one apply settles it, since + // applying cannot resize the split view. If the split view + // WAS resized, applying synchronously from inside the + // resize's own layout pass fights AppKit, so re-arm one + // deferred apply against the settled size instead (below). + // We cannot just park and wait for the host: a host whose + // per-pane ideals are container-independent re-imposes the + // SAME extent, and only when its own inputs change — an + // apply may never terminate off-target without a re-arm + // edge, or the divider stays at the proportional position + // indefinitely. And never apply when the divider is already + // at the target: a same-position setPosition still runs a + // layout pass, which re-applies surface sizes, which // re-imposes — a once-per-turn churn loop at full CPU. let moved = abs(current - (lastImposedOutcome ?? .infinity)) > 0.01 let availUnchanged = abs(available - (lastImposedAvail ?? -1)) <= 0.01 @@ -1124,6 +1129,16 @@ struct SplitContainerView: NSViewRepresentabl } else { imposedRetryBudget = 0 } + } else if moved && !availUnchanged { + // The container resized under an unchanged imposed + // extent. Recording the new avail immediately bounds + // this to one re-arm per size change; the deferred + // apply runs a turn later, after AppKit's resize pass + // has finished, so there is no recursive fighting. + lastImposedAvail = available + imposedRetryBudget = max(imposedRetryBudget, 1) + imposedRetrySplitView = splitView + scheduleImposedRetry() } else if imposedRetryBudget > 0 { // A retry chain the drag-session gate interrupted parked // here with budget left; the session-end renudge lands in @@ -1369,16 +1384,28 @@ struct SplitContainerView: NSViewRepresentabl // isSyncingProgrammatically, so the recursive didResize is // caught by the guard above; waiting a turn would let the // in-between frame reach ghostty and reflow content). A - // split with an imposed extent must NOT do that: when its - // container resizes, the stored extent is out of date, and - // putting the divider back from inside the very layout - // pass that moved it starts another layout pass — that - // recursion is what pinned the main thread. The host sends - // a new extent for the new size; until it lands, a - // not-yet-right frame is fine. + // split with an imposed extent must NOT do that: putting + // the divider back from inside the very layout pass that + // moved it starts another layout pass — that recursion is + // what pinned the main thread. But it cannot just park + // and wait for the host either: a host whose per-pane + // ideals are container-independent re-imposes the SAME + // extent, and only when its own inputs change, so nothing + // would ever move the divider off AppKit's proportional + // position. Re-arm one deferred apply against the settled + // size instead — recording the new avail immediately + // bounds this to one re-arm per size change, and the + // apply runs a turn later, outside this layout pass. A + // mid-drag retry refuses without consuming the budget and + // the session-end renudge resumes the chain. if self.splitState.imposedFirstExtent == nil { let statePosition = self.splitState.dividerPosition self.syncPosition(statePosition, in: splitView) + } else if abs(availableSize - (self.lastImposedAvail ?? -1)) > 0.01 { + self.lastImposedAvail = availableSize + self.imposedRetryBudget = max(self.imposedRetryBudget, 1) + self.imposedRetrySplitView = splitView + self.scheduleImposedRetry() } self.onGeometryChange?(false) return diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index 7c076910..124502a2 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -1062,14 +1062,16 @@ public final class BonsplitController { /// Repeating the extent stays cheap when the divider is already at the /// target (the coordinator refreshes its memos without a layout pass). /// - /// The extent is applied when set (and re-applied if the divider drifts - /// while the split view's own size is unchanged), but it is NOT - /// re-asserted after the split view resizes: a stored extent computed - /// for the old size is stale at the new one, and re-applying it from - /// inside the resize's layout pass fights AppKit recursively. A host - /// that imposes extents is expected to impose fresh values when the - /// container it derived them from changes; until it does, the divider - /// rides AppKit's proportional resize. + /// The extent is applied when set, re-applied if the divider drifts + /// while the split view's own size is unchanged, and re-applied once + /// (deferred a runloop turn, clamped to the new bounds) after the split + /// view itself resizes. The resize case cannot apply synchronously — + /// that fights AppKit from inside its own layout pass — but it cannot + /// just wait for the host either: a host whose per-pane ideals are + /// container-independent re-imposes the same extent, and only when its + /// own inputs change, so a divider parked at AppKit's proportional + /// position would stay there until some unrelated input. An apply never + /// terminates off-target without re-arming on the next size change. public func setImposedFirstExtent( _ extent: CGFloat?, forSplit splitId: UUID, fromExternal: Bool = false ) -> Bool { @@ -1086,9 +1088,9 @@ public final class BonsplitController { // coordinator's renudge path is memo-only when the divider is // already at the target, so this cannot churn layout; when the // divider drifted (a container resize rescaled it while the - // re-planned extent came out identical), this is the only thing - // that puts it back — see `syncPosition`'s "bounded by explicit - // calls" contract. + // re-planned extent came out identical), this puts it back + // without waiting for the coordinator's own resize re-arm — + // see `syncPosition`'s "bounded by explicit calls" contract. split.imposedEpoch &+= 1 } split.syncDividerNow?() diff --git a/Tests/BonsplitTests/BonsplitTests.swift b/Tests/BonsplitTests/BonsplitTests.swift index b8cd80a6..858ecfe2 100644 --- a/Tests/BonsplitTests/BonsplitTests.swift +++ b/Tests/BonsplitTests/BonsplitTests.swift @@ -3532,12 +3532,11 @@ final class BonsplitTests: XCTestCase { /// resizes, and the fresh plan often computes the SAME number: per-pane /// ideals are container-independent (a row of N terminal cells is the /// same points in any container). Meanwhile AppKit's proportional resize - /// has moved the divider off the imposed extent. The coordinator's - /// renudge path documents that every explicit imposition call re-arms - /// one apply ("bounded by explicit calls"), so the identical re-imposition - /// must put the divider back — deduping it by value leaves the divider - /// wedged at the proportional position until some input happens to - /// change the number. + /// has moved the divider off the imposed extent. The coordinator gives + /// the parked divider one bounded apply of its own when the container + /// settles, and the identical re-imposition must also be accepted, not + /// deduped by value — either way the divider ends at the extent, never + /// wedged at the proportional position. @MainActor func testReimposingSameExtentAfterContainerResizeRetargetsExactly() throws { var configuration = BonsplitConfiguration() @@ -3598,26 +3597,28 @@ final class BonsplitTests: XCTestCase { let imposed = CGFloat(120) XCTAssertEqual(settleImposed(imposed), imposed, accuracy: 1.5) - // The container shrinks; AppKit rescales the split proportionally and - // the divider drifts off the imposed extent (one-writer: nothing puts - // it back until the host re-imposes). + // The container shrinks; AppKit rescales the split proportionally, + // moving the divider off the imposed extent. The resize itself + // re-arms one deferred apply, so the divider comes back without a + // fresh impose call. window.setContentSize(NSSize(width: 300, height: 300)) for _ in 0..<12 { contentView.layoutSubtreeIfNeeded() RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + if abs(splitView.arrangedSubviews[0].frame.width - imposed) <= 1 { break } } - let drifted = splitView.arrangedSubviews[0].frame.width - XCTAssertLessThan( - drifted, - imposed - 4, - "expected the proportional resize to move the divider off the imposed extent" + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + imposed, + accuracy: 1.5, + "the container resize must re-arm one apply of the stored extent" ) XCTAssertEqual( settleImposed(imposed), imposed, accuracy: 1.5, - "identical re-imposition after a container resize was deduped away; the divider stayed at the drifted position" + "an identical re-imposition after a container resize must be accepted, not deduped away" ) } @@ -3708,6 +3709,91 @@ final class BonsplitTests: XCTestCase { ) } + /// A container resize rescales the split proportionally, moving the + /// divider off its imposed extent. Hosts whose per-pane ideals are + /// container-independent re-impose the SAME extent, and only when their + /// own inputs change — so if nothing re-applies the stored extent here, + /// the divider stays at the proportional position indefinitely. The + /// imposed sync must give the parked divider one bounded apply against + /// the settled size, with no fresh imposition and no drag session. + @MainActor + func testImposedExtentReappliesAfterContainerResizeWithoutFreshImposition() throws { + var configuration = BonsplitConfiguration() + configuration.appearance.minimumPaneWidth = 1 + configuration.appearance.minimumPaneHeight = 1 + configuration.appearance.enableAnimations = false + configuration.dividerPositionRange = 0...1 + let controller = BonsplitController(configuration: configuration) + _ = controller.createTab(title: "Left") + let sourcePane = try XCTUnwrap(controller.focusedPaneId) + XCTAssertNotNil(controller.splitPane( + sourcePane, + orientation: .horizontal, + initialDividerPosition: 0.5 + )) + guard case .split(let split) = controller.treeSnapshot() else { + XCTFail("Expected split root") + return + } + let splitId = try XCTUnwrap(UUID(uuidString: split.id)) + + let hostingView = NSHostingView( + rootView: BonsplitView(controller: controller) { _, _ in + Color.clear + } emptyPane: { _ in + Color.clear + } + ) + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + let contentView = try XCTUnwrap(window.contentView) + hostingView.frame = contentView.bounds + hostingView.autoresizingMask = [.width, .height] + contentView.addSubview(hostingView) + window.makeKeyAndOrderFront(nil) + contentView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.05)) + contentView.layoutSubtreeIfNeeded() + let splitView = try XCTUnwrap(firstDescendant(ofType: NSSplitView.self, in: hostingView)) + XCTAssertEqual(splitView.arrangedSubviews.count, 2) + + let imposed = CGFloat(120) + XCTAssertTrue(controller.setImposedFirstExtent(imposed, forSplit: splitId)) + for _ in 0..<12 { + splitView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + contentView.layoutSubtreeIfNeeded() + if abs(splitView.arrangedSubviews[0].frame.width - imposed) <= 1 { break } + } + XCTAssertEqual(splitView.arrangedSubviews[0].frame.width, imposed, accuracy: 1.5) + + // Grow the container. No further imposition, no drag session: the + // stored extent is the only authority left, and it still fits. + window.setContentSize(NSSize(width: 640, height: 300)) + for _ in 0..<12 { + contentView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + splitView.layoutSubtreeIfNeeded() + if abs(splitView.arrangedSubviews[0].frame.width - imposed) <= 1 { break } + } + XCTAssertGreaterThan( + splitView.bounds.width, + 500, + "the window resize must have reached the split view for this test to mean anything" + ) + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + imposed, + accuracy: 1.5, + "a parked imposed divider must get one bounded apply when the container settles" + ) + } + /// Drag-end promises the settled geometry has already been reported when /// `splitTabBarDividerDragDidEnd` runs. A `fromExternal` update opens a /// short suppression window for outgoing geometry notifications; a From b2a7f4a99e5bc1eb4bc9779ffe2d6626c5246fff Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 16:16:06 -0700 Subject: [PATCH 09/10] Prove the drift and isolate the re-impose recovery in the resize tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both container-resize tests only checked the final divider width, so they would pass even if AppKit never moved the divider off the imposed extent. Capture the divider synchronously after setContentSize, before any runloop turn — the heal is deferred a turn, so the proportional drift (192pt for the 400 to 640 grow, under 116pt for the shrink) is observable there — and only then pump and assert the recovery. The same-extent re-imposition test also lost its teeth: the size-change re-arm already restores the extent before the re-impose call runs, so the final assertion could no longer tell whether the call was accepted or deduped by value. Isolate it by perturbing the divider at constant container size with a direct AppKit setPosition (the imposition stays stored and no avail changes, so the size-change re-arm cannot fire), pumping with no impose call and asserting the divider stays put — nothing heals it autonomously — then re-imposing the identical extent and asserting the divider returns. The recovery is attributable to the explicit call alone. --- Tests/BonsplitTests/BonsplitTests.swift | 82 ++++++++++++++++++++----- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/Tests/BonsplitTests/BonsplitTests.swift b/Tests/BonsplitTests/BonsplitTests.swift index 858ecfe2..e593777c 100644 --- a/Tests/BonsplitTests/BonsplitTests.swift +++ b/Tests/BonsplitTests/BonsplitTests.swift @@ -3532,11 +3532,13 @@ final class BonsplitTests: XCTestCase { /// resizes, and the fresh plan often computes the SAME number: per-pane /// ideals are container-independent (a row of N terminal cells is the /// same points in any container). Meanwhile AppKit's proportional resize - /// has moved the divider off the imposed extent. The coordinator gives - /// the parked divider one bounded apply of its own when the container - /// settles, and the identical re-imposition must also be accepted, not - /// deduped by value — either way the divider ends at the extent, never - /// wedged at the proportional position. + /// has moved the divider off the imposed extent. Two recovery edges must + /// both work, and this test pins each to its own trigger: the container + /// size change re-arms one apply of its own, and an explicit identical + /// re-imposition must be accepted, not deduped by value. The second half + /// isolates the re-imposition by moving the divider at CONSTANT + /// container size — the size-change re-arm cannot fire there, and the + /// test proves nothing heals the divider until the re-impose call runs. @MainActor func testReimposingSameExtentAfterContainerResizeRetargetsExactly() throws { var configuration = BonsplitConfiguration() @@ -3597,11 +3599,22 @@ final class BonsplitTests: XCTestCase { let imposed = CGFloat(120) XCTAssertEqual(settleImposed(imposed), imposed, accuracy: 1.5) - // The container shrinks; AppKit rescales the split proportionally, - // moving the divider off the imposed extent. The resize itself - // re-arms one deferred apply, so the divider comes back without a - // fresh impose call. + // The container shrinks; AppKit rescales the split proportionally. + // Capture the divider before any runloop turn — the re-arm's heal is + // deferred a turn, so the drift must be observable here or the + // recovery assertions below would pass vacuously. window.setContentSize(NSSize(width: 300, height: 300)) + contentView.layoutSubtreeIfNeeded() + splitView.layoutSubtreeIfNeeded() + let drifted = splitView.arrangedSubviews[0].frame.width + XCTAssertLessThan( + drifted, + imposed - 4, + "expected the proportional resize to move the divider off the imposed extent" + ) + + // The size change itself re-arms one deferred apply: the divider + // comes back with no fresh impose call. for _ in 0..<12 { contentView.layoutSubtreeIfNeeded() RunLoop.current.run(until: Date().addingTimeInterval(0.02)) @@ -3614,11 +3627,39 @@ final class BonsplitTests: XCTestCase { "the container resize must re-arm one apply of the stored extent" ) + // Now isolate the re-imposition edge from the size-change edge: move + // the divider at CONSTANT container size, behind the coordinator's + // back (a direct AppKit setPosition keeps the imposition stored and + // changes no avail, so the size-change re-arm cannot fire). + let perturbed = CGFloat(200) + splitView.setPosition(perturbed, ofDividerAt: 0) + splitView.layoutSubtreeIfNeeded() + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + perturbed, + accuracy: 1.5, + "the programmatic perturbation must actually move the divider" + ) + + // With no size change and no impose call, nothing may heal this: + // pumping must leave the divider where the perturbation put it. + for _ in 0..<6 { + splitView.layoutSubtreeIfNeeded() + RunLoop.current.run(until: Date().addingTimeInterval(0.02)) + contentView.layoutSubtreeIfNeeded() + } + XCTAssertEqual( + splitView.arrangedSubviews[0].frame.width, + perturbed, + accuracy: 1.5, + "at constant container size, no autonomous heal may move the divider — recovery below is attributable to the re-impose call alone" + ) + XCTAssertEqual( settleImposed(imposed), imposed, accuracy: 1.5, - "an identical re-imposition after a container resize must be accepted, not deduped away" + "an identical re-imposition must be accepted, not deduped away — it is the only actor left" ) } @@ -3774,18 +3815,29 @@ final class BonsplitTests: XCTestCase { // Grow the container. No further imposition, no drag session: the // stored extent is the only authority left, and it still fits. + // Capture the divider before any runloop turn — the heal is deferred + // a turn, so the proportional drift must be observable here or the + // recovery assertion below would pass vacuously. window.setContentSize(NSSize(width: 640, height: 300)) + contentView.layoutSubtreeIfNeeded() + splitView.layoutSubtreeIfNeeded() + XCTAssertGreaterThan( + splitView.bounds.width, + 500, + "the window resize must have reached the split view for this test to mean anything" + ) + XCTAssertGreaterThan( + splitView.arrangedSubviews[0].frame.width, + imposed + 20, + "expected the proportional resize to move the divider off the imposed extent (~192pt for 400→640)" + ) + for _ in 0..<12 { contentView.layoutSubtreeIfNeeded() RunLoop.current.run(until: Date().addingTimeInterval(0.02)) splitView.layoutSubtreeIfNeeded() if abs(splitView.arrangedSubviews[0].frame.width - imposed) <= 1 { break } } - XCTAssertGreaterThan( - splitView.bounds.width, - 500, - "the window resize must have reached the split view for this test to mean anything" - ) XCTAssertEqual( splitView.arrangedSubviews[0].frame.width, imposed, From 92093ae3a46b26c77bf5eb562711db2e054c0739 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Mon, 13 Jul 2026 21:34:28 -0700 Subject: [PATCH 10/10] Let a host app take over debug log appends dlog appended to its log file by opening a FileHandle per line and doing seekToEnd()+write. When another writer in the same process appends to the same file, the two race: lines interleave, clobber each other, and land out of timestamp order. A host app can now install an external sink that receives every line, so all appends can go through one serialized writer. Without a sink, appends keep a single O_APPEND handle open instead of reopening per line, and dump() steps aside entirely when a sink owns the file. --- Sources/Bonsplit/Public/DebugEventLog.swift | 54 +++++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/Sources/Bonsplit/Public/DebugEventLog.swift b/Sources/Bonsplit/Public/DebugEventLog.swift index 9e0edda1..eafff206 100644 --- a/Sources/Bonsplit/Public/DebugEventLog.swift +++ b/Sources/Bonsplit/Public/DebugEventLog.swift @@ -11,6 +11,28 @@ public final class DebugEventLog: @unchecked Sendable { private let queue = DispatchQueue(label: "cmux.debug-event-log") private static let logPath = resolveLogPath() + /// When set, lines are handed to this closure instead of appended here. + /// A host app whose own debug log writes to the same file installs a + /// sink at startup so the file has exactly one serialized append path; + /// two independent appenders interleave and reorder lines under load. + /// Confined to `queue`. + private var externalSink: (@Sendable (String) -> Void)? + + /// Fallback appender for standalone use (no sink installed): one handle + /// opened with O_APPEND and kept open, so a concurrent appender cannot + /// clobber whole lines. Confined to `queue`. + private var appendHandle: FileHandle? + + /// Routes every subsequent line to `sink` (or back to the built-in file + /// append when `nil`). The sink receives the raw message, without the + /// timestamp prefix, so the receiving log applies its own line format. + public static func setExternalSink(_ sink: (@Sendable (String) -> Void)?) { + let log = shared + log.queue.async { + log.externalSink = sink + } + } + private static let formatter: DateFormatter = { let f = DateFormatter() f.dateFormat = "HH:mm:ss.SSS" @@ -55,29 +77,43 @@ public final class DebugEventLog: @unchecked Sendable { public func log(_ msg: String) { let ts = Self.formatter.string(from: Date()) - let entry = "\(ts) \(msg)" queue.async { + if let sink = self.externalSink { + sink(msg) + return + } + let entry = "\(ts) \(msg)" if self.entries.count >= self.capacity { self.entries.removeFirst() } self.entries.append(entry) // Append to file for real-time tail -f - let line = entry + "\n" - if let data = line.data(using: .utf8) { - if let handle = FileHandle(forWritingAtPath: Self.logPath) { - defer { try? handle.close() } - guard (try? handle.seekToEnd()) != nil else { return } - try? handle.write(contentsOf: data) - } else { - FileManager.default.createFile(atPath: Self.logPath, contents: data) + guard let data = (entry + "\n").data(using: .utf8) else { return } + if self.appendHandle == nil { + let fd = open(Self.logPath, O_WRONLY | O_APPEND | O_CREAT, 0o644) + if fd >= 0 { + self.appendHandle = FileHandle(fileDescriptor: fd, closeOnDealloc: true) } } + guard let handle = self.appendHandle else { return } + do { + try handle.write(contentsOf: data) + } catch { + // The file may have been rotated away; reopen on the next line. + self.appendHandle = nil + } } } /// Write all buffered entries to the log file (full dump, replacing contents). public func dump() { queue.async { + // With a sink installed the receiving log owns the file; replacing + // its contents here would throw away the other writer's lines. + if self.externalSink != nil { return } + // The atomic write replaces the inode; reopen the handle lazily. + try? self.appendHandle?.close() + self.appendHandle = nil let content = self.entries.joined(separator: "\n") + "\n" try? content.write(toFile: Self.logPath, atomically: true, encoding: .utf8) }