Port round 2 from upstream cmux: close race, scroll boost, monitor identity - #150
Merged
Conversation
Adapted from upstream cmux regression 49a0cd8559: right after a terminal is created, no shell state has been reported and no TTY is known, so there is nothing to lose and closing must not prompt. The ghostty needs-confirm fallback can spuriously return true in that window. testUnknownActivityWithoutTTYNeverConfirms fails on purpose; the fix lands in the next commit so CI proves the test catches the race.
Ported from upstream cmux 2e03978ae1, adapted to our shell-integration telemetry: upstream gates on ghostty_surface_foreground_pid/tty_name, which our ghostty fork does not export, so we gate the unknown-activity fallback on surfaceTTYNames instead — no confirmation until a TTY has been reported for the panel. promptIdle/commandRunning behavior is unchanged, and once a TTY is known the ghostty fallback is honored as before.
) Ported from upstream cmux da4e4bc460. Trackpads and Magic Mouse drive a gesture phase and keep the historical 2x precise-delta boost; high-res free-spin wheels report precise deltas with empty phase/momentumPhase and were getting the boost stacked on top of the OS's own wheel acceleration, producing runaway scrolling. The boost decision now lives in GhosttyTerminalScrollBoost, a pure struct with unit coverage; the precision bit forwarded to ghostty_surface_mouse_scroll is unchanged.
) Ported from upstream cmux 7a06b39766 plus the geometry tie-break from 021379f054. macOS can reassign CGDirectDisplayID numbers across monitor unplug/replug and sleep/wake, so restoring by raw displayID could land windows on the wrong monitor or off-screen. Display snapshots now also persist a CGDisplayCreateUUIDFromDisplayID-backed stableID; restore matches on it first, tie-breaking duplicates by geometry, and falls back to the old displayID matching for snapshots saved before this change. Not ported from the upstream cluster (0e7447ac2c, 31b9021448, the capture-guard half of 021379f054): they patch a per-configuration frame ring and mirror-signature system this fork never had.
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
1 participant
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.
What this does
Second round of fixes brought over from upstream cmux, same shape as #130. Three user-visible annoyances go away: a spurious "close tab?" prompt when closing a terminal that just opened, runaway scrolling with high-res mouse wheels, and restored windows landing on the wrong monitor after unplugging a display or sleep/wake.
Closes #135.
Summary
Nine upstream commits were analyzed against our tree; three produced real changes, six were not applicable because they patch upstream features this fork never had. Every port is an adaptation, not a raw cherry-pick.
Ported:
2e03978ae1close-confirmation-before-tty-attach race. Upstream gates on ghostty C API exports (foreground_pid/tty_name) our ghostty fork doesn't have; we gate the unknown-activity fallback on our ownsurfaceTTYNamestelemetry instead. Lands as two commits per the regression policy: failing test first, fix second.da4e4bc460precise-scroll boost gating. OurscrollWheeldoubled precise deltas unconditionally — the exact upstream bug. The decision now lives in a pureGhosttyTerminalScrollBooststruct with four-case unit coverage; only gesture-phase devices get the 2x boost. The wheel classifier is unit-tested; the real-device behavior (high-res mouse vs trackpad) still deserves a manual spin.7a06b39766+ the geometry tie-break from021379f054: stable per-monitor identity (CGDisplayCreateUUIDFromDisplayID) persisted alongside displayID, matched first on restore, geometry tie-break for duplicates, full fallback for old snapshots (field is Optional; pre-existing sessions decode fine).Not applicable (recorded so the next port round doesn't re-litigate):
0e7447ac2cmirror-state display signatures and31b9021448frame-ring sanitization: patch a per-configuration frame-memory system (windowConfigFrames/SessionConfigFramePolicy) we never built.021379f054: no continuous capture guard exists here.b0f7b0bf89link-through-panel-mapping: ourTerminalPanel.idis structurally identical to its surface id, so the id-space bug can't occur; the deep-link feature it protects doesn't exist here.b0cfd2cbb5/856d255728hover-close reconciliation and493d5ad707closure capture: both fix upstream's AppKit list-with-cell-reuse sidebar; ours is SwiftUI with stable per-workspace identity and no reuse.Test Plan
WorkspaceCloseConfirmationTests(4, red-then-green across the two commits),GhosttyScrollPreciseBoostTests(4), two stable-identity cases inSessionPersistenceTestsSessionPersistenceTestssuite (54) green — old-snapshot compatibility coveredtest_workspace_split_title_updates.pygreen against it (same files as the close-race fix)