Repository navigation
Mirrored tmux panes render exactly their assigned spans, sized by a single transaction - #7938
Merged
austinywang merged 85 commits intoJul 13, 2026
Merged
Conversation
Pull the mirror's divider-fraction walk out of the bonsplit applier into RemoteTmuxNativeSplitLayout.plan, a pure function from (measured tree, metrics, container size) to per-split fractions and per-pane outer sizes, modeling the native split view's whole-point division. The mirror now just zips the plan onto the bonsplit tree. Add a seeded fuzz that drives random layouts and containers through the claim (clientGrid), a tmux-style cell assignment, and this exact walk, then derives each pane's rendered grid from its outer size the way the terminal surface does. It asserts every pane renders at least its assigned span — one column short means every full-width line in that pane wraps. The fuzz FAILS on this commit: proportional ideal-over-ideal fractions let whole-point rounding compound down the binary split chain, so in a near-exact container the deepest panes come up short. The fix follows.
Panes in a mirrored tmux window could render narrower than the width tmux assigned them. A pane even one column short wraps every full-width line, so multi-column layouts looked shredded; a window whose claimed size exactly filled the container garbled every pane at once. The terminal surface floors its size to whole cells, so a pane's pixels must never come out below its assigned span — but pane sizes were made by translating exact point targets into normalized divider fractions, and the fraction machinery cannot carry that precision: its drift deadbands eat sub-1% changes (two or three columns at terminal sizes), rounding compounds down the binary split chain, and the claim reserved no margin, so every pane sat exactly on the boundary where one lost point costs one column. Stop translating. Bonsplit now accepts an imposed first-child extent in points per split and applies it verbatim: clamped only by pane minimums, the equivalent fraction mirrored back for ratio readers, cleared by a user drag, and convergence memo-based so a target AppKit refuses cannot spin the main thread. The plan computes each split's extent directly — assigned cells times cell size plus measured chrome, including tmux's own pane title rows when pane-border-status is active — scaled evenly when the container genuinely cannot fit, quantized upward to the device-pixel grid with an axis-tagged running remainder so error never accumulates with depth or leaks across axes. The claim leaves one device pixel per axis unclaimed: exactly the quantum round-up can accumulate to, not a tuning constant. Container resizes re-impose the plan. DEBUG observability so harnesses ask instead of guessing with timers: a remote.grid.mismatch log line whenever a pane settles on a grid different from its assignment, and a remote.tmux.sizing_settled socket verb reporting per visible window whether sizing settled and which panes fall short. The native layout fuzz from the previous commit goes green: random trees, metrics, and containers — including exact-fit and one-extra-cell regimes — every pane renders at least its assigned span, degradation is even, and the other axis stays exact.
RemoteTmuxMirrorGeometry.clientCells/frames and the mirror's framesForRender had no callers outside their own tests: the native chrome rewrite replaced the live pipeline with RemoteTmuxNativeLayoutMetrics for the claim and bonsplit divider fractions for the render, but left the old walk in the tree with doc comments still describing it as the sizing authority. That cost us a real debugging detour — the comments point at the wrong code when the mirror misrenders. Keep the struct itself: it carries the measured render constants (cell/padding/scale) the native metrics are built from, plus the claim floors. Drop the dead functions, their frames value type, their test suite, and the one feed-forward test that exercised the dead entry point, and rewrite the mirror's header to describe the pipeline that actually runs. The one still-live function the deleted suite covered (patchingLeafRects) keeps its regression test.
A window larger than its own display is never a state a user created — interactive resizing and zoom are both display-bounded — yet exact-frame preservation restored such frames verbatim whenever the saved display still matched. A layout feedback bug (fixed separately in this branch) once persisted a 13,000-point-wide window, and every launch after that restored the giant frame and re-poisoned everything derived from window geometry. Restore now clamps a saved frame's size to its display's visible frame; position handling is unchanged.
A sync pass lays out hosted split views and writes the host frame, and the notifications those emit can be delivered after the pass ends, so no in-pass reentrancy flag catches them all. The echo then re-runs the sync forever on identical geometry, pinning the main thread. Each pass now fingerprints everything it reads or writes — window size, container, reference, host, and hosted frames — and an incoming pass with an identical fingerprint is a no-op, so echoes die in one cheap comparison while any real change still syncs fully.
Sizing work now happens in exactly one place, at most once per runloop turn, and only when its inputs changed. Every trigger — container geometry, tmux layouts, calibration samples, visibility, title rows — writes its data and requests a pass; nothing runs layout directly. The pass claims, plans, and applies once against a snapshot of the inputs, and events that fire during it (including the samples and geometry callbacks our own applies produce) can only update data and request a follow-up. The follow-up stops when inputs stopped changing: feedback converges by fixed point, bounded by real input changes. This replaces five accumulated anti-feedback guards — an imposition epoch nudge, a same-target distance check, refusal retry budgets, a plan-input gate, and a geometry fingerprint on the external sync path — each of which suppressed one edge of the same producer-consumer cycle. A design that needs that many guards is describing the loop it wishes it did not have; this one cannot loop, because event handlers cannot start work. Container sizes are also clamped to the largest attached display at the recording boundary: nothing displayable exceeds a screen, so no honest container does either, whichever view leaks a content-derived ideal.
An anchor geometry callback ran a synchronous full-portal sync — hierarchy layout, every hosted view, plus a deferred follow-up — and those callbacks fire for every layout pass, including the passes the sync itself runs. Under pane churn that kept the display cycle busy indefinitely (the main thread sat at full CPU inside portal sync with the app otherwise idle). Outside a live drag, anchor callbacks now coalesce into the scheduled pass like every other trigger; drags keep the immediate path so the dragged split stays visually glued. The sync fingerprint now includes each anchor's expected rect, so passes keep running while any hosted view disagrees with its anchor and stop exactly when aligned. DEBUG builds expose the disagreement list, the sizing-settled probe reports it per window, and the live fuzz fails on it: a hosted terminal drawn over tab strips or dividers is now a first-class gate failure even when every grid is exact.
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR refactors remote tmux mirror sizing around coalesced sizing transactions, native split-layout planning, per-window claim tracking, reconnect revalidation, and portal geometry synchronization. It also adds display-aware window-frame capping, debug settlement reporting, expanded tests, and remote fuzz/reproduction scripts. ChangesRemote tmux layout and sizing
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 21❌ Failed checks (1 warning, 20 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
Three code paths computed 'where should this hosted view be' three ways: the frame writer used the ancestor-clipped, pixel-snapped anchor rect, while the sync fingerprint and the misplacement judge both used the raw anchor conversion. For a clipped anchor those disagree, so the judge could flag a correct frame and the fingerprint never settled. All three now share one function. A sync request arriving for the portal whose own pass is on the stack was dropped as echo — but a pass's layout can produce genuinely new geometry: an imposed divider correction rides the pass's layoutSubtreeIfNeeded, and its notification lands mid-pass. Dropping it left the final correction unapplied forever (the last hosted write predated the whole settle window). Mid-pass requests now mark a follow-up the pass schedules on exit, mirroring the sizing transaction's rule that events during a drain update state and request a pass. Termination is unchanged: a follow-up that finds geometry matching the fingerprint does no layout and emits nothing. An entry whose visibleInUI flips on also schedules a sync: a view shown again may still hold the frame it was born with. The live fuzz's storm test and a surface frame tripwire ride along in DEBUG builds.
Review findings, each with a concrete failure. The sizing fingerprint covered only the merged rendered tree, but the claim reads the BASE tree — its residual depends on the full tree even while zoomed — so a base change hiding behind an unchanged visible tree skipped the pass and left tmux on a stale size through a whole settle window (the live fuzz caught the matching claimed-vs-layout wedge independently). Base and visible trees are now fingerprinted separately. The window's size bound is the largest attached display rather than the current one: a restore can legitimately target a bigger screen, and AppKit moves the window after sizing it. A hidden mirror's first container measurement is recorded only when usable — a hidden mount can report 0x0 first, and recording that consumed the one unvalidated slot the initial claim needs, blocking every later measurement. And the settled probe now flags panes rendering more than one cell BEYOND their span (content drawn over chrome), not just shortfalls, with the live fuzz failing on either.
The surplus flag added for a review finding false-positived on the first fuzz seed: a full-height pane beside a stack of tab-barred siblings legitimately inherits their chrome as blank fill margin — several cells of grid surplus with a perfectly placed view. Overdraw is a property of the VIEW, not the grid, and the anchor-misplacement entries already judge it exactly (they caught a real three-point overdraw the grids could not see). The settled probe keeps shortfall as the only grid defect and documents why.
The keep-alive stack sized itself to its LARGEST mounted workspace. A hidden workspace never lays out smaller — so the maximum only ever ratchets up — and the selected workspace stretches to fill the inflated union. One point of internal overgrowth anywhere in any workspace then becomes permanent, window-wide, compounding growth: observed live at five thousand points inside a 1,728-point window, growing three and a half points per frame at rest, with the display clamps containing the claims it fed. Every mounted workspace now gets the container's exact size from a GeometryReader — workspaces are pages, and a page renders at its container's size regardless of what its content momentarily thinks it needs. DEBUG builds gain an anchor-side chain dump (the SwiftUI half of the portal geometry) that fires alongside misplacement reports — walking that chain from the growing pane's anchor is what named this mechanism.
The gate judges steady-state churn, but iteration one could begin while the initial claim/layout handshake was still converging — back-to-back seeds (teardown, then reconnect) legitimately take tens of seconds to settle, and those attach transients read as sizing failures when every following iteration is clean. The harness now waits for one settled report before the op loop starts and prints the attach latency as its own measurement. Ruler checks also re-read twice under multi-window load, where a two-second redraw loop lags further behind.
Contributor
Author
|
The clamp of the window size is okay belts and suspenders, but what is fundamentally causing the window to need grow at all? It seems like it means we didn't tell tmux the proper available dims in the first place, causing too much pressure that pushing the window wide? |
A size requested while the connection is attaching was recorded but never sent, and deduping retries against the request table then suppressed every resend of a size the server never saw. Requests and sends are now separate ledgers: dedup asks what the server has, the request table remains the claim ledger and reconnect reseed source, and a reconnect clears the sent table because the fresh client has been sent nothing.
…ndows tmux derives window sizes from client sizes, and a fresh control client after a server restart sits at 80 columns — per-window pins alone left every window wedged near the default no matter what was claimed. The claim path now keeps the session-wide client size at the running maximum of live window claims, and a window tmux removes takes its size-table entries with it: stale entries from dead ids (server restarts reuse low ids) replayed obsolete pins on reconnect and dragged the client floor to sizes no live window claims.
A mirror could outlive its window: a window killed while the transport was down loses its close event in the gap, and the corpse then claims, replans, and gets judged against a window that no longer exists — it can never settle. Reconciliation now tears down any mirror whose id is absent from the live window list even when panel bookkeeping already lost it, and the settled probe skips mirrors whose window is not listed. The remaining half — refetching the full window snapshot on reattach instead of trusting event continuity across gaps — follows.
Recording before the send let an attempt made while the transport was down masquerade as delivered — the dedup ledger then suppressed every retry of a size the server never received. DEBUG builds also log every refresh-client send with the connection state, so the send side of any future sizing investigation is evidence rather than inference.
The ghost-window investigation needed the id-level sequence of close events versus snapshot applications; these two lines make any future topology question readable straight from the log.
The marathon runs every seed against one long-lived app, and each seed's fresh-lab setup kills the tmux server. A workspace left mounted from a prior seed then points at a server that was killed and recreated with recycled window ids, and its reconnect churns without converging — which is a reconnection-robustness concern, not the steady-state sizing this gate exists to measure. Each seed now closes the workspace it opened, so seeds are independent and the gate measures what it claims to.
…time A mismatch or unsettled window read when the 20s poll expires may be mid-transition: an end-of-seed relayout storm or a reconnect can leave a window seconds from convergence. The gate measures state at rest, so it now polls a final stretch and fails only on a state that stays wrong — logging the extra convergence time so a window that always needs the reconfirm is visible as its own slow-to-settle signal rather than silently tolerated.
tmux accepts multiple -B directives per refresh-client, so a pane's reflow, cwd, and header subscriptions now go out as one command instead of three. Under rapid pane churn the per-pane subscription sends dominate the control stream, and collapsing 3->1 keeps the command FIFO from backing up faster than tmux drains it — the difference between the stream keeping pace and stalling into non-convergence.
A mirror's container cannot exceed the content area of the window hosting it. SwiftUI can briefly hand the sizing callback a content-derived width when an ancestor adopts a layout ideal — seen at fresh connect with a starved pane, where the container read the full display width while the app window was a third of it, so the claim spiked to the display ceiling and tmux, sized to the real window, never matched it and wedged. The container now clamps to the hosting window's content width when a visible window holds the panes, and once a size is on record it defers an unvalidated reading (no visible window yet) rather than banking a stale full-display measurement. The largest display remains the fallback only for the first-ever attach measurement, when no window exists to bound against.
… pass A first container measurement taken before the hosting window was visible (fresh connect) banks a display-width fallback, and if the container's point size never changes again no later geometry callback corrects it — the claim stays at the display ceiling and tmux, sized to the real window, never matches, wedging the window. Each sizing pass now re-clamps the stored container to the live window's content width before it runs, so the next pass after the window appears shrinks the claim to the truth and re-claims, without depending on another callback.
An imposition applies to bonsplit on the next runloop turn, so anchors move after the pass returns. The portal syncs hosted views from AppKit's async geometry callbacks, which under churn can sample an anchor before its imposed move or coalesce the catch-up away, leaving a hosted view at a stale wider frame over its shrunk neighbor (a one-column pane drawing several columns over its sibling). The pass now schedules a portal resync explicitly, two turns out so the apply has landed — the transaction owns the geometry change, so it owns telling the portal rather than racing notifications.
Extends the FIFO dequeue logging to the per-window-size claim so a sizing investigation can see tmux's own accept/reject of each refresh-client claim, not just that it was sent. A controlled shrink-then-grow of the app window confirms the claim path is honored end to end (err=0, window lands at the claimed size); this logging is what proved it and remains for diagnosing the intermittent churn+reconnect race.
A workspace reconnect during active churn could leave tmux holding a window at a size that went stale across the transport gap: the reseed replays the cached per-window sizes, but a container change that raced the outage makes that cache wrong, and tmux keeps the window there. On the reconnect's connected edge, every visible mirror now runs a fresh sizing pass, whose in-pass container re-validation re-derives the claim from the live window — so the post-reconnect size is current truth, not a replayed stale value. Isolated by the live fuzz's reconnect op, which this makes converge.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
@coderabbitai review |
✅ Action performedReview finished.
|
austinywang
enabled auto-merge (squash)
July 13, 2026 16:06
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmux.xcodeproj/project.pbxproj (1)
4634-4638: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the dangling RemoteTmux layout references at cmux.xcodeproj/project.pbxproj:4634-4638
RemoteTmuxLayoutContainer.swift,RemoteTmuxImposedFrameLayout.swift,RemoteTmuxDividerStrip.swift, andRemoteTmuxWeightedSplitLayout.swifthave no matching source files orPBXFileReference/PBXBuildFileentries anywhere in the repo. Add the actual files and wire them into the target, or drop these stale UUIDs; as-is, the project points at nonexistent sources and will break Xcode loading/builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux.xcodeproj/project.pbxproj` around lines 4634 - 4638, Remove the stale entries for RemoteTmuxLayoutContainer.swift, RemoteTmuxImposedFrameLayout.swift, RemoteTmuxDividerStrip.swift, and RemoteTmuxWeightedSplitLayout.swift from the Xcode project, including any corresponding PBXFileReference or PBXBuildFile UUIDs if present. Do not alter the valid RemoteTmuxPendingLayout.swift entry.Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeSplitLayoutPlanner.swift (1)
133-139: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAccumulate
outerSizesin one traversal.Each split recursively creates two dictionaries and then allocates another dictionary via
merging. For a skewed tree withPpanes, this can become O(P²) copying during a sizing pass. Use a single inout accumulator instead.Proposed fix
public func outerSizes(of plan: Plan) -> [Int: CGSize] { + var result: [Int: CGSize] = [:] + appendOuterSizes(of: plan, to: &result) + return result +} + +private func appendOuterSizes(of plan: Plan, to result: inout [Int: CGSize]) { switch plan { case .leaf(let paneId, let outer): - guard let paneId, let outer else { return [:] } - return [paneId: outer] + if let paneId, let outer { + result[paneId] = outer + } case .split(_, _, _, let first, let second): - return outerSizes(of: first).merging(outerSizes(of: second)) { first, _ in first } + appendOuterSizes(of: first, to: &result) + appendOuterSizes(of: second, to: &result) } }As per path instructions, avoid repeated full-collection scans and unbenchmarked slower algorithms over scalable collections.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeSplitLayoutPlanner.swift` around lines 133 - 139, Update outerSizes(of:) to use a single inout dictionary accumulator during one recursive traversal, rather than recursively creating dictionaries and combining them with merging. Preserve the existing leaf behavior and first-value precedence for duplicate pane IDs, using a private helper if needed while keeping the public outerSizes(of:) API unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/remote-tmux-fuzz-lock.sh`:
- Around line 8-42: The lock acquisition logic around the mkdir loop must never
reclaim directories with missing or malformed owner metadata. Require a valid
numeric pid and corresponding token before considering a lock initialized;
otherwise fail closed with the existing refusal status. Reclaim only fully
initialized locks whose recorded process is proven dead, and make stale-lock
cleanup ownership-safe and atomic so the original creator cannot remove or write
through a lock directory acquired by another driver. Apply the same safeguards
to the cleanup logic around the later lock-release section.
In `@Sources/Debug/RemoteTmux/RemoteTmuxControlConnection`+TestSupport.swift:
- Around line 1-29: Remove the DEBUG-only RemoteTmuxControlConnection extension
and its test-only APIs from production Sources. Widen only the underlying
declarations needed by tests to internal, then recreate equivalent helpers in
cmuxTests using `@testable` import, including access to pending command/layout
state and message handling without exposing additional production seams.
---
Outside diff comments:
In `@cmux.xcodeproj/project.pbxproj`:
- Around line 4634-4638: Remove the stale entries for
RemoteTmuxLayoutContainer.swift, RemoteTmuxImposedFrameLayout.swift,
RemoteTmuxDividerStrip.swift, and RemoteTmuxWeightedSplitLayout.swift from the
Xcode project, including any corresponding PBXFileReference or PBXBuildFile
UUIDs if present. Do not alter the valid RemoteTmuxPendingLayout.swift entry.
In
`@Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeSplitLayoutPlanner.swift`:
- Around line 133-139: Update outerSizes(of:) to use a single inout dictionary
accumulator during one recursive traversal, rather than recursively creating
dictionaries and combining them with merging. Preserve the existing leaf
behavior and first-value precedence for duplicate pane IDs, using a private
helper if needed while keeping the public outerSizes(of:) API unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: edf44462-5428-47ba-80a0-014561e854c1
📒 Files selected for processing (45)
Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxLayoutNode.swiftPackages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeLayoutMetrics.swiftPackages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeMeasuredSplitTree.swiftPackages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxNativeSplitLayoutPlanner.swiftPackages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/RemoteTmux/Layout/RemoteTmuxPaneTitleRowPlacement.swiftPackages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteTmuxNativeLayoutMetricsTests.swiftSources/App/CmuxMainWindow.swiftSources/AppDelegate+WindowFramePolicy.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/RemoteTmuxControlConnection+TestSupport.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/Debug/RemoteTmux/TerminalWindowPortal+DebugDiagnostics.swiftSources/GhosttyTerminalView.swiftSources/RemoteTmuxConnectionObservers.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+Commands.swiftSources/RemoteTmuxControlConnection+Diagnostics.swiftSources/RemoteTmuxControlConnection+LayoutPublication.swiftSources/RemoteTmuxControlConnection+Observation.swiftSources/RemoteTmuxControlConnection+Sizing.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxSessionMirror+Sizing.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxWindowMirror+Bonsplit.swiftSources/RemoteTmuxWindowMirror+BonsplitLayout.swiftSources/RemoteTmuxWindowMirror+DividerSizing.swiftSources/RemoteTmuxWindowMirror+SizingTransaction.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalWindowPortal.swiftSources/WorkspaceContentView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppDelegateOversizedFrameRestoreTests.swiftcmuxTests/RemoteTmuxConnectionWindowSizingTests.swiftcmuxTests/RemoteTmuxMirrorFeedForwardTests.swiftcmuxTests/RemoteTmuxMirrorLayoutMathTests.swiftcmuxTests/RemoteTmuxNativeMirrorLayoutFuzzTests.swiftcmuxTests/RemoteTmuxRectPublicationTests.swiftcmuxTests/RemoteTmuxWindowMirrorFocusSeedTests.swiftcmuxUITests/RemoteTmuxSizingUITests+Lab.swiftcmuxUITests/RemoteTmuxSizingUITests.swiftscripts/remote-tmux-fuzz-host.shscripts/remote-tmux-fuzz-lock.shscripts/remote-tmux-fuzz-marathon.shscripts/remote-tmux-layout-repro.sh
💤 Files with no reviewable changes (2)
- Sources/RemoteTmuxControlConnection+Diagnostics.swift
- cmuxUITests/RemoteTmuxSizingUITests+Lab.swift
Comment on lines
+8
to
+42
| while ! mkdir "$CMUX_FUZZ_LOCK_DIR" 2>/dev/null; do | ||
| if [ -L "$CMUX_FUZZ_LOCK_DIR" ] || [ ! -d "$CMUX_FUZZ_LOCK_DIR" ] \ | ||
| || [ ! -O "$CMUX_FUZZ_LOCK_DIR" ]; then | ||
| echo "fuzz lock exists but is not an owned directory — refusing to start" >&2 | ||
| return 96 | ||
| fi | ||
| holder="$(cat "$CMUX_FUZZ_LOCK_DIR/pid" 2>/dev/null)" | ||
| case "$holder" in ''|*[!0-9]*) holder="" ;; esac | ||
| if [ -n "$holder" ] && kill -0 "$holder" 2>/dev/null; then | ||
| echo "another fuzz driver (pid $holder) is running — refusing to start" | ||
| return 96 | ||
| fi | ||
| if [ "$attempts" -ge 1 ]; then | ||
| echo "could not atomically acquire fuzz lock" >&2 | ||
| return 96 | ||
| fi | ||
| # Stale takeover removes only this protocol's private entries. `rmdir` | ||
| # refuses a caller-supplied directory containing anything else. | ||
| rm -f -- "$CMUX_FUZZ_LOCK_DIR/pid" "$CMUX_FUZZ_LOCK_DIR/token" | ||
| if ! rmdir -- "$CMUX_FUZZ_LOCK_DIR" 2>/dev/null; then | ||
| echo "owned stale fuzz lock contains unexpected entries — refusing to remove it" >&2 | ||
| return 96 | ||
| fi | ||
| attempts=$((attempts + 1)) | ||
| done | ||
|
|
||
| CMUX_FUZZ_LOCK_TOKEN="$$.$RANDOM.$(date +%s)" | ||
| if ! printf '%s\n' "$$" > "$CMUX_FUZZ_LOCK_DIR/pid" \ | ||
| || ! printf '%s\n' "$CMUX_FUZZ_LOCK_TOKEN" > "$CMUX_FUZZ_LOCK_DIR/token"; then | ||
| rm -f -- "$CMUX_FUZZ_LOCK_DIR/pid" "$CMUX_FUZZ_LOCK_DIR/token" | ||
| rmdir -- "$CMUX_FUZZ_LOCK_DIR" 2>/dev/null | ||
| echo "could not initialize fuzz lock" >&2 | ||
| return 96 | ||
| fi | ||
| CMUX_FUZZ_LOCK_OWNED=1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not reclaim an uninitialized lock directory.
Between mkdir on Line 8 and writing pid on Lines 35-36, another driver sees an empty holder, removes the directory as “stale,” and acquires it. The original driver can then write into that new lock and both runs proceed concurrently. Treat missing/malformed owner metadata as an in-progress/unknown lock and fail closed; reclaim only a fully initialized lock whose recorded owner is proven dead, with ownership cleanup made atomic.
Also applies to: 65-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/remote-tmux-fuzz-lock.sh` around lines 8 - 42, The lock acquisition
logic around the mkdir loop must never reclaim directories with missing or
malformed owner metadata. Require a valid numeric pid and corresponding token
before considering a lock initialized; otherwise fail closed with the existing
refusal status. Reclaim only fully initialized locks whose recorded process is
proven dead, and make stale-lock cleanup ownership-safe and atomic so the
original creator cannot remove or write through a lock directory acquired by
another driver. Apply the same safeguards to the cleanup logic around the later
lock-release section.
Comment on lines
+1
to
+29
| #if DEBUG | ||
| extension RemoteTmuxControlConnection { | ||
| func installStdinWriterForTesting(_ writer: RemoteTmuxControlPipeWriter) { | ||
| stdinWriter = writer | ||
| } | ||
|
|
||
| func handleMessageForTesting(_ message: RemoteTmuxControlMessage) { | ||
| handle(message) | ||
| } | ||
|
|
||
| var pendingCommandKindsForTesting: [RemoteTmuxControlCommandKind] { | ||
| pendingCommands | ||
| } | ||
|
|
||
| func hasPendingSizingSettlementWork(windowId: Int) -> Bool { | ||
| if pendingLayouts[windowId] != nil { return true } | ||
| return pendingCommands.contains { command in | ||
| switch command { | ||
| case .paneRects(let pendingWindowId, _), .perWindowSize(let pendingWindowId): | ||
| return pendingWindowId == windowId | ||
| case .listWindows: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move these test seams out of the production target.
This DEBUG extension exposes private connection state solely for tests. Remove it, make only the required declarations internal, and implement test helpers in cmuxTests using @testable import.
As per coding guidelines and path instructions, production Sources/** must not add test-only/debug-only seams; tests should access minimally widened internal declarations from the test target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Debug/RemoteTmux/RemoteTmuxControlConnection`+TestSupport.swift
around lines 1 - 29, Remove the DEBUG-only RemoteTmuxControlConnection extension
and its test-only APIs from production Sources. Widen only the underlying
declarations needed by tests to internal, then recreate equivalent helpers in
cmuxTests using `@testable` import, including access to pending command/layout
state and message handling without exposing additional production seams.
Sources: Coding guidelines, Path instructions
This was referenced Jul 16, 2026
This was referenced Jul 17, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Panes in a mirrored tmux window could render narrower than the width tmux assigned them — a pane even one column short wraps every full-width line — and under sustained pane churn the app could climb to full CPU and beach-ball. To reproduce the first, mirror a six-column window whose panes each run a full-width ruler (
scripts/remote-tmux-layout-repro.shbuilds one); for the second, run the live fuzz below for a few minutes.Depends on manaflow-ai/bonsplit#176 (the submodule bump in this PR). Supersedes #7931.
Root cause
Two layers. The sizing arithmetic translated exact point targets into normalized divider fractions, and the fraction machinery cannot carry that precision: sub-1% changes are damped to stop drift, and 1% of a large split is several terminal columns. Underneath that, sizing work could start from any event handler — container geometry, tmux layouts, calibration samples, portal callbacks — and each apply emits more of those events, so consumers of geometry were also producers of it. That cycle is what pinned the CPU: layout passes re-triggering themselves through notifications delivered after any guard flag was already down.
Dogfooding an eight-window mirror session exposed a third layer, on the render side. Four authorities write pane geometry — SwiftUI proposals, AppKit split view layout, bonsplit's own model, and the portal's forced window layout — with no rule about who owns it when. When a container changed under an imposed split, bonsplit re-asserted its now-stale extent from its resize delegate, inside the very layout pass that had moved the frames; the re-assert ran another layout pass, which moved more frames, recursively across every mounted split. A main-thread sample during the resulting beach-ball showed 100% of the time in that fight. Divider drags failed for the mirror image of the same missing rule: drag-end was detected only when a resize callback happened to arrive while the mouseUp was still the current event, so a drag-pause-release reconciled nothing, and any sizing pass landing mid-drag re-imposed over the gesture and swallowed its
resize-pane.Fix
The arithmetic is a pure plan: the claim tells tmux how many cells fit (container minus measured chrome), tmux assigns every pane's span, and each split boundary's absolute position is rounded to the nearest whole point — so every boundary lands within half a point of exact at any depth, every pane within one point of ideal, which the claim's one point of per-pane slack covers. Bonsplit applies the planned extents verbatim through an imposed-extent API.
Sizing runs as a single transaction: triggers may only write data and request a pass, one coalesced pass per runloop turn does claim, plan, and apply against a snapshot of the inputs, and it re-runs only while inputs changed — feedback converges by fixed point, with no retry budgets or event dedup anywhere. The portal's anchor callbacks coalesce the same way outside live drags. Windows and recorded containers are clamped to their displays, so no content-derived size can compound through anything, and session restore clamps saved frames for the same reason.
On the render side, exactly one authority writes a split's geometry at any moment. At rest the plan owns it: bonsplit no longer re-asserts a held extent when frames move under it — a moved frame means the container changed and the extent is stale, so the correction is the next pass re-imposing from fresh inputs, never bonsplit re-applying the old value mid-layout. Mid-drag the user owns it: a divider drag is now a real session bracketed by the divider's mouse-tracking lifecycle (begin at mouseDown on the divider, guaranteed end when the tracking loop returns at release), sizing passes hold for the length of the session, and drag-end converts the final extent to cells, sends
resize-pane, and lets tmux's reply settle the layout. Hidden tabs get no writes at all, and the portal skips their surfaces during bulk geometry syncs — a mirror session keeps dozens of hidden surfaces, and recomputing each one's clipped frame on every tick of a live window resize made resizing visibly sluggish.Tests
The unit fuzz drives random trees, metrics, and four container regimes through the real claim and plan, asserting every pane derives at least its assigned span in the renderer's own integer arithmetic — the points→cells floor now lives in one production helper the tests share, so the model and the implementation cannot drift on the floor-vs-round question, and a dedicated case pins tmux title rows (one reserved cell-height per pane) through the same exact-fit round trip. The ownership rule has its own coverage: a sizing pass firing mid-drag must hold and be consumed at session end, the drag-session counter survives imbalance, and — against the real renderer in a real window — a container change under a held imposition must not be fought until a fresh imposition retargets it. In the app, a debug probe logs any pane that settles off its assignment, and a debug socket verb reports per window whether sizing settled and which panes mismatch — including hosted terminal views whose frames drifted off their anchors, so content drawn over tab strips or dividers fails the gate even when every grid is exact.
scripts/remote-tmux-fuzz-host.shstands up the whole "remote" side locally (a loopback sshd whose logins land in an isolated tmux server), andscripts/remote-tmux-live-fuzz.shdrives the running app through seeded random churn against it, with a marathon wrapper for long unattended runs.scripts/remote-tmux-shape-zoo.shbuilds an adversarial eight-window session (nested stacks, six columns, single pane, grids) whose panes each run a live width ruler, for hands-on dogfood.Verification
Unit suites are green (28 tests across the three sizing suites, including the ownership cases). Against the live zoo session: an adversarial sweep — tab churn across all eight layouts, socket-driven window resizes, tmux-side layout flips — settles to a quiet log (about ten diagnostic lines per 20 seconds, versus thousands per second when the render loop was live) with tmux coherent afterwards (every window's panes tile its pinned size exactly). Divider drags reconcile end to end: each drag session logs mouseDown → begin → end →
resize-panesent, and tmux's reply re-imposes the settled layout. The previously worst fuzz seeds run clean with the app idling at 2% CPU. One open finding from the new overlap detector (a hosted view three points wider than its anchor at settle) predates the ownership rule and has not reproduced since; the marathon below re-judges it.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Mirrored tmux panes now render exactly their assigned cell spans. Sizing and redraw run as one debounced, host‑bound transaction against the window’s live geometry across reconnects and hidden views.
New Features
RemoteTmuxNativeSplitLayoutPlannerwith public layout types inCmuxRemoteSession;vendor/bonsplitbumped for imposed‑extent.remote.tmux.sizing_settledover the control socket and portal misplacement diagnostics; isolated local repro + live fuzz + marathon/lock harnesses; test runner uses explicit tmux capture flags for stable text checks.Bug Fixes
sizeThatFitsthe proposal; workspaces render at the container’s size; windows refuse programmatic growth and restore clamps oversized saved frames; nested divider drags use applied extents to avoid mid‑drag drift; added tests for clamped restore and reconnect sizing.Written for commit 36f4b9e. Summary will update on new commits.
Edge cases the gate found, and what fixed them
The genuinely odd ones — each of these was invisible to unit tests and took the live gate plus targeted instrumentation to name:
NSApp.currentEventinside a resize callback — so a drag that paused before release, or any release with no coincident resize, never reconciled, and stale mouseUps made unrelated programmatic resizes log phantom drag-ends. The drag is now a session bracketed by the divider's own mouse-tracking lifecycle, which cannot miss the release.And the ordinary bugs worth confessing because they cost real time: a shell RNG that silently froze when called in subshells (the fuzzer reported green while doing nothing),
sort -Rbreaking seed reproducibility, failure counts wrapping past 255 into success exit codes, and a marathon whose pid lock could be defeated by deleting the pid file — it now holds the lock for the run's duration and standalone fuzz runs refuse to start alongside it.The earlier open finding — a pane view growing without bound at a few points per frame at rest — matches the render-side re-assertion loop named above and has not reproduced since the one-writer rule landed; the growth-hunt instrumentation stays in place until the marathon re-certifies it.
Live fuzz marathon
Baseline run 2026-07-13 (render-ownership build, before the workspace-pane fix below): 20 seeds x 25 iterations against the running app mirroring a real tmux server — 713 iterations, 213 failures, zero hangs, zero crashes. The failures split into three signatures:
NSHostingView<Bonsplit.PaneContainerView>— the workspace-level pane hosting — while the claim-side clamps hold, so tmux stays sane and the damage is confined to render distortion. This is the long-open "pane view grows at rest" finding, now located; fix queued.Note
High Risk
Large refactor of remote-tmux sizing, window geometry, and tmux control transport on the main thread; regressions could mis-size sessions, wedge claims after reconnect, or still allow layout/portal drift despite new guards.
Overview
Fixes mirrored tmux windows rendering one or more columns short of tmux’s assigned span (full-width line wrap) and runaway CPU from geometry handlers re-triggering layout.
Sizing model: Drops the old device-pixel
frames()/ fraction imposition path. Layout math moves intoCmuxRemoteSession(RemoteTmuxNativeSplitLayoutPlanner, metrics with title-row and quantization slack). The mirror claims tmux cells from the container, then plans and applies whole-point split extents via Bonsplit’ssetImposedFirstExtent, with drag sync ignoring ratio noise while an imposition is active.Single transaction: All triggers only update inputs and call
setNeedsSizingPass; one main-queue pass snapshots inputs, pushesupdateClientSize, andimposeDividerPlan, then stops when inputs are unchanged. Container sizes are clamped to the hosting window (and display on first measure); hidden mirrors only get an initial claim. Reconnect forces visible mirrors to re-size; dead windows drop size claims and orphan mirrors.Transport / shell: Per-window sizing dedupes against
sentWindowSizes(cleared on reconnect); session client size tracks the max window claim; pane live subscriptions batch to onerefresh-client.CmuxMainWindowcaps programmatic frame growth; session restore clamps oversized saved frames. TerminalssizeThatFitsto the proposal so SwiftUI cannot inflate the window.DEBUG / tests: Adds
remote.tmux.sizing_settled(claim vs layout, grid shortfalls, plan vs view, portal anchor drift) and portal misplacement stats; unit tests cover the debug dispatch and planner pipeline.Reviewed by Cursor Bugbot for commit 5e5a806. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests