Repository navigation
base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (from #33120) - #33377
Merged
Merged
Conversation
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 3, 2026 08:25 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Shipped in:
|
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 3, 2026 08:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (independent-subagent): SHIP at 45112a7 Checked:
Not run: the tests themselves. |
lawrencecchen
added
the
ready-to-land
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…argument table is nonisolated Cherry-picked so CmuxNextAppTests compiles for this branch's fleet steps: since #33120 the test target does not compile at feat-cmux-next on the Linux compile-check (job f88be3b39d1e at 6078fb3) or on the Mac lane (step c4cad3cdb743 at feat 43f59d3): "main actor-isolated static property 'cases' cannot be accessed from outside of the actor" (@test(arguments:)). #33377 lands the same one-line change on feat-cmux-next; merging feat after it lands makes this commit a no-op. (cherry picked from commit 45112a7) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…(cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7)
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…h builds
FRAME OWNER REFACTOR S0. The fleet films and benches Release builds with
bench hooks (CMUX_BENCH_HOOKS). Their launch never turns the tunable store
on (DebugSettingsService.start() needs DevTools: a Debug compile or a
nightly bundle), so `debug.tunables set` changed nothing there and
`state` said active false. One build could not film a Debug switch off and
on, and the speed sweep could not scale the springs.
`debug.tunables {"action": "activate"}`: on a store that launch left off,
it registers TunableCatalog.all and calls activate(file: nil), so overrides
apply for this launch, in memory only; on a store that is already on (DEV,
NIGHTLY) it does nothing. It answers `state`. It exists only where the
method does (#if DEBUG || CMUX_BENCH_HOOKS), and nothing changes unless a
client calls it, so other benches stay the same.
Memory only: nothing is written and the next launch starts off again (the
next launch would not read a file, and a stale one could change another
owner's bench). plans/cmux-next/debug-settings.md (Gate) now says what a
`set` after `activate` reaches: per-read tunables (a switch read per
frame, MotionSpring.base) at their next read, a reader that tracks the key
after `activate`, not one that tracked it before; launch-time readers
(dragResize's synced choice, layerHosting at the next page show,
icons.family, auth.signInWindow) do not follow.
Red: DebugTunablesActivateTests at 9316d72 (feat 1a7e2c1
merged into the test commit 4ee6acb, which does not build its tests on
its own base: the RowBadgeSpinnerDensityTests base red #33377 fixed).
Green: the same suite at this commit. Command for both: cmux-ci run --class
isolated --script scripts/ci/package-test-lane.sh --arg=suite
--arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha>.
The fleet step ids and results are in the pull request body.
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…unable store on (#33553) * debug.tunables: the tunable store is a parameter (no behavior change) DebugTunables.handle and DebugTunables.state take the store they read and write (default TunableStore.shared, which the socket method and every other caller still use). A test can then drive a store of its own: other suites in the test process activate TunableStore.shared, so the shared store cannot show a store that launch left off (FRAME OWNER REFACTOR S0, the next commits). * test: debug.tunables activate turns on a store that launch left off (red) FRAME OWNER REFACTOR S0. The fleet's film builds are Release builds with bench hooks (CMUX_BENCH_HOOKS). Their launch never turns the tunable store on (DebugSettingsService.start() needs DevTools: a Debug compile or a nightly bundle), so `debug.tunables set` changes nothing there and `state` says active false (film round px-col-drag-2 on FEAT DOGFOOD BUILD 1d8e759e). One build then cannot film a Debug switch off and on, and the speed sweep cannot scale the springs. New file DebugTunablesActivateTests (a store of its own per test: other suites turn TunableStore.shared on, and a store never turns off): - without `activate`, a `set` changes nothing and `state` says off (passes on the base too: benches that never call activate stay the same); - `activate` turns the store on with no override file, and each later `set` reaches the next read of a switch and of the `move` spring; - a reader that tracks a tunable after `activate` is told of the next `set`; - `activate` keeps a store that is already on (DEV, NIGHTLY) as it is. Expected on this commit: the last three fail ("unknown action"; the value and `active` do not change). * debug.tunables activate: turn the tunable store on at runtime in bench builds FRAME OWNER REFACTOR S0. The fleet films and benches Release builds with bench hooks (CMUX_BENCH_HOOKS). Their launch never turns the tunable store on (DebugSettingsService.start() needs DevTools: a Debug compile or a nightly bundle), so `debug.tunables set` changed nothing there and `state` said active false. One build could not film a Debug switch off and on, and the speed sweep could not scale the springs. `debug.tunables {"action": "activate"}`: on a store that launch left off, it registers TunableCatalog.all and calls activate(file: nil), so overrides apply for this launch, in memory only; on a store that is already on (DEV, NIGHTLY) it does nothing. It answers `state`. It exists only where the method does (#if DEBUG || CMUX_BENCH_HOOKS), and nothing changes unless a client calls it, so other benches stay the same. Memory only: nothing is written and the next launch starts off again (the next launch would not read a file, and a stale one could change another owner's bench). plans/cmux-next/debug-settings.md (Gate) now says what a `set` after `activate` reaches: per-read tunables (a switch read per frame, MotionSpring.base) at their next read, a reader that tracks the key after `activate`, not one that tracked it before; launch-time readers (dragResize's synced choice, layerHosting at the next page show, icons.family, auth.signInWindow) do not follow. Red: DebugTunablesActivateTests at 9316d72 (feat 1a7e2c1 merged into the test commit 4ee6acb, which does not build its tests on its own base: the RowBadgeSpinnerDensityTests base red #33377 fixed). Green: the same suite at this commit. Command for both: cmux-ci run --class isolated --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha>. The fleet step ids and results are in the pull request body. * debug.tunables activate: the doc names the readers a bench set cannot reach (review) Review of 3be0406 (independent subagent, FIX on the doc only): - browserResize.dragResize's synced choice: Chromium is the default engine and ChromiumWarmup starts it about 3 s after launch at the first idle moment, so activate and set must come before that, not "before the first Chromium page". - A reader that tracked a key before activate never hears of a change: SidebarTunablesWatch keeps its launch snapshot (8 row values) until a relaunch (only sidebar.rowCloseSpot follows), and an ObservationStream loop that started before stays where it is. - icons.family: agent pages made after the set use it. - Readers gated on DevTools.isEnabled or wired only by DebugSettingsService.start() keep their Release behavior in a bench build: auth.signInWindow, home.chiefViewGallery, sidebar.sections.localPrototype, home.flightRecorder and its captures. Test: the `file` null check proved nothing (it is DebugSettingsService's file, which only start() sets, null on the red ref too) and is gone. It passed on the red ref, so which tests fail there does not change. * debug.tunables activate: the doc says when Chromium really starts (second review) Second independent review (of 90b21bb, FIX on the doc; no code defect): - Every cmux page shows in Chromium by default (browser.defaultEngine, agent.chromium), and the launch's New Tab spare page (NewTabSparePool, 250 ms after the first frame) starts Chromium through PageWebView.startChromium, so CEFRuntime reads dragResizeAtLaunch then, normally before a socket client has sent `activate`. The "about 3 s" ChromiumWarmup window was wrong: a bench build cannot count on reaching the `synced` choice with `activate`; `debug.cef` (`state`, `trigger`) shows when Chromium started. - The first New Tab after a `set` takes the spare page made at launch, which keeps the old icon family. - An observation made before `activate` stays silent only when it reads tunables alone; one that also reads other state fires for that state and follows from then on. This commit changes only plans/cmux-next/debug-settings.md. Fleet proof of the branch (FRAME OWNER S0), command for each: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=DebugTunablesActivateTests --ref <sha> --key <lane>-<sha12> (through proof-step.py; the controller takes this script only as class light): - Red at 9316d72 (feat 1a7e2c1 merged into the test commit 4ee6acb, no fix): step 36c6e3dff7bf61a7f25ca21e (cmux10) FAILED as expected, 3 of 4 tests: `activate` answers "unknown action"; `active`, browserResize.pageBackdrop and the `move` spring do not change after a `set`; the reader that tracks after `activate` is told 0 times. withoutActivateASetChangesNothing passed, as it must on both sides. - Green at 3be0406 (the fix): step a122187239af62861d765bd1 (aws-m4pro-5), 4 of 4 passed. Green at 90b21bb (this commit's parent: the same Swift as this commit): step f1ac080a739723faf9edf6e6 (aws-m4pro-3), 4 of 4 passed. - Compile-checks (cmux-ci compile-check --app --package CmuxNext --recipe-version 2): 9316d72 passed (817e15f36602047c4f49687e), 3be0406 passed (8faf68223e4eefd3abda2a69), 90b21bb passed (997c8ae58d61ba6be177c60c). * debug.tunables activate: three doc nits from the third review Third independent review (SHIP at 9145aba; these were nits): - Not every cmux page shows in Chromium: the update island and the icon picker require WebKit (transparent overlay). The doc names New Tab and the agent chats, the pages that start Chromium early. - The launch spare is taken by a New Tab, by a new workspace's first page (takeSpare) and by an existing chat of this Mac (takeConnectedSpare), not only by New Tab; each keeps the old icon family. - debug.cef's `state` and `trigger` say whether and why Chromium started; `ready_after_launch_s` says when. Only plans/cmux-next/debug-settings.md changes; the Swift is 90b21bb's.
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…me tick (behind motion.frameOwner.timing, off) (#33725) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…ld-layout frame (D1, regression of #32635) (#33607) * test(resize keys): one key press sends each terminal one grid, the final one (D1, red) FRAME OWNER REFACTOR S3a. D1 is a regression of #32635 (RESIZE KEYS K3): the sigwinch TUI counts 24 SIGWINCH for 8 key presses on builds with #32635 and 8 without it, and film rk-browser-agent (build 52788515, frame 269) shows the program's start layout (35 columns) for one frame between 38-column frames. The test puts the app's own pane content (PaneContentView, a tab strip over its content host) with a real terminal (TerminalSession, libghostty surface) in each pane of p1 | p2, presses Ctrl-Shift-H or Ctrl-Shift-L on p2, steps the spring to its landing at a fixed 120 Hz dt, runs the landing's after-commit release (LayoutViewContext.afterCommit) and one more layout pass. Each terminal must ask its PTY for the final grid once, at the press, and nothing at the landing, and its surface must never take its start size again after the press. Expected on this commit (no fix): each terminal asks for the start grid and then the final grid again at the landing, and the surface takes its start size at the release (PaneHostView.releaseHeldContent sets only the pane content's frame; endPaneMotion -> releaseSurfaceHold sizes the surface from the terminal host, which still has the start size). Command: cmux-ci run --class isolated --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests --red --ref <this commit> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(resize keys): the one-grid red test covers Ctrl-Shift-K and J too (D1, red) The film's 8 presses include the up and down keys: the same rig with p1 over p2 (a vertical split) presses resizePaneUp and resizePaneDown on p2. Still no fix: every case is expected to fail the same way (start grid, then the final grid again, at the landing; the surface takes its start size at the release). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * base red (from #33377, not landed yet): RowBadgeSpinnerDensityTests' argument table is nonisolated Cherry-picked so CmuxNextAppTests compiles for this branch's fleet steps: since #33120 the test target does not compile at feat-cmux-next on the Linux compile-check (job f88be3b39d1e at 6078fb3) or on the Mac lane (step c4cad3cdb743 at feat 43f59d3): "main actor-isolated static property 'cases' cannot be accessed from outside of the actor" (@test(arguments:)). #33377 lands the same one-line change on feat-cmux-next; merging feat after it lands makes this commit a no-op. (cherry picked from commit 45112a7) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(pane motion): the other two releases of held content send no second grid (D1, red) FRAME OWNER REFACTOR S3a, the refactor owner's review: one test for each other path that lets held content go through PaneHostView.releaseHeldContent. - KeyboardResizeOneGridReportTests.aPaneACloseLetsGoAtRestDuringASecondPressSendsNoSecondGrid: endKeyResizeHold at rest. p1 | (p2 over p3); Ctrl-Shift-L on p1 lands, and before the run loop next waits (where that frame commits and lets go of what it held) Ctrl-Shift-J on p2 springs p2 and p3 and a close of p3 arrives. The close lets go of p1 at rest. - PaneHeldContentLandingTests (new file): motion.pane.holdContent on (set in the shared store, put back after, serialized suite), a host whose size comes to rest a frame before its origin: the landing frame only moves it. Each terminal must send no grid after the final one its spring's start sent, and its surface must never take its start size again. Expected on this commit (no fix): the start grid, then the final grid again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(pane motion): the D1 red tests compile (stored property order, main-actor helper) Compile-check 3ad37799f8ed (ffadef1) failed on two lines of the new tests: Rig set its after-commit closure, which captures self, before its `panes` property was set; and the held-content test's local grid reader was not main-actor isolated inside the generic helper's closure. No change to what the tests check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(resize keys): a held pane content lays out at its rest size before it hears the landing (D1) FRAME OWNER REFACTOR S3a. D1 is a regression of #32635 (RESIZE KEYS K3): one Ctrl-Shift-H/J/K/L press sent each terminal's PTY three grids (final, start, final): the sigwinch TUI counted 24 SIGWINCH for 8 presses on builds with #32635 and 8 without it, and film rk-browser-agent (build 52788515, frame 269) showed the program's start layout for one frame between final-grid frames. PaneHostView.releaseHeldContent set only the pane content's frame (layoutClip), then told the content the motion settled. The pane content (PaneContentView) frames its content host in layout(), so the content host and the terminal host in it kept the start size until the next layout pass; the terminal let its surface hold go from that stale host (start size, start grid) and the next pass sent the final grid again. A Chromium page heard the landing at its stale frame the same way. The release now lays the host's subtree out (a real size in a window only, the rule applyPresentation's per-frame layout follows) before notifyPaneMotionSettled, so every view in the content has its rest frame when it hears the landing. The one content layout is the same work as before, in the same after-commit pass, before the notification instead of after it. All three releases go through this function: the K3 key-resize release after the landing frame's commit, endKeyResizeHold for a pane at rest, and a spring's held content (motion.pane.holdContent, agent panes) at its landing frame. Red, at 94db280 (no fix): step 3789fb93581a3a6a26245771 (aws-m4pro-5), KeyboardResizeOneGridReportTests and PaneHeldContentLandingTests FAILED: 4 of 4 keys and both terminals sent the start grid and the final grid again at the landing, the surface took its start size again (e.g. 540 -> 600 -> 540 pt); the endKeyResizeHold and the holdContent landing tests failed the same way. Earlier red of the key test alone: step 47b106dfc7b2a3d5a929a39e at a6b34db. Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests,PaneHeldContentLandingTests --ref <sha> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(resize keys): a close that takes over a key spring sends the taken-over terminal one grid (D1, red) Review of #33607 (FIX at 519815d): PaneHostView.endKeyResizeHold for a pane still on its key spring sets only the pane content's frame (layoutClip); in the same turn the structural change's applyPresentation calls announceRest, and TerminalSurfaceView.restSize adds the size change to the terminal host's stale held size. The terminal sends a wrong "final" grid at the change's start and holds a wrong surface size; the landing then sends the right grid: two PTY resizes for one split or close during a key spring. aCloseDuringASecondPressSendsEachTerminalOnlyItsFinalGrid (the endKeyResizeHold test, renamed) now also checks p2, the pane the close takes over on the second press's spring (6 frames in): exactly one grid for the close, its final one, and the surface rests at its host size. Expected on this commit (no fix for that branch): p2 sends the wrong grid and then the final one. Also from the review: PaneHeldContentLandingTests injects the hold (PaneHostView.init(holdsContentInMotion: { true })) instead of setting motion.pane.holdContent in the shared TunableStore: the app's hosts get the provider's closure and never read that tunable, and the shared override could leak to suites running at the test's await points. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(resize keys): a pane a structural change takes over mid key spring lays out before the change measures it (D1) Review of #33607 (FIX at 519815d). PaneHostView.endKeyResizeHold, for a pane still on its key spring when a split, close or zoom takes the screen over, set only the pane content's frame (layoutClip). In the same turn the structural change's presentation calls announceRest, and the terminal adds the size change to its host's size (TerminalSurfaceView.restSize), which the key hold had kept: it sent a wrong "final" grid at the change's start, held its surface at that wrong size, and sent the right grid at the landing. Both letting-go paths now share one helper (layOutLetGoContent): layoutClip, then the subtree's layout at a real size in a window, before the content hears anything (notifyPaneMotionSettled for a pane at rest, announceRest for a pane the change takes over). PaneHostView.swift is one line shorter than on the base. Red, at f1f7ae0 (the takeover test, no fix for that branch): step bb3b972222294053e40f7a0f (aws-m4pro-6) FAILED as expected: p2, which a close took over 6 frames into a Ctrl-Shift-J spring, asked its PTY for 73x43 (its size before the first press plus the close's change) and then 66x44; every other test passed (the three paths 519815d fixed). Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests,PaneHeldContentLandingTests --ref <sha> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
… their curve after a late frame (#33935) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. * Frame owner S2 ON: motion.frameOwner.timing defaults on (springs at each frame's show time, drags once per frame tick) Prepared on PR #33725's head; merge feat after #33725 lands, before the PR opens. The film verdict and the typing-load-l6 pair go in the PR body. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…mit (behind motion.frameOwner.terminals, off) (#33956) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * test(next): a terminal resize commit never carries a layer size other than its frame's (red, FRAME OWNER REFACTOR S4) Stage S4 (terminals ready-or-keep, PIXEL-PERFECT PLAN.md): for every terminal in every commit, the layer's size and its pixels change together, or neither changes. Films P2: a host-sized Ghostty surface's view took its new bounds at once and the frame drawn at that size came 2-4 refreshes later, so those commits showed the old frame in the new bounds. TerminalSurfaceCommitSizeTests (new file, CmuxNextAppTests): a real MANUAL_MIRROR surface in a window whose layout() resizes it in AppKit's display cycle; an observer right after Core Animation's commit (CommitBoundary.order) samples, at the end of every main run-loop turn, the surface layer's bounds in pixels and the IOSurface it shows. No clock, no sleep: it waits for commits. - aResizeCommitNeverCarriesALayerSizeOtherThanItsIOSurface: six drag steps; no turn ends with the two at different sizes; the last size shows its own frame. - aReadyFrameIsPresentedInTheCommitThatChangesItsBounds: the first turn with the new bounds shows the frame of that size, and host_fresh_presents grew (the frame was presented in a commit, not by the renderer later). Seam with no effect yet: the Debug switch motion.frameOwner.terminals (TerminalDragTunables.framesOwnedByCommit, default off), which the tests turn on. Expected red on feat: the bounds change before the frame. * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. * GhosttyNextKit: pin 635c6c12a (ghostty-next #69 + #70: the embedder presents a new host size with its bounds) Moves the pin from 34e73ba8b to the release branch merge commit 635c6c12a (release/cmux-9ce9b7d0-e2 = 34e73ba8b + fork PR #69 + fork PR #70): GhosttyNextKit xcframework-635c6c12aaa7d5db895bf8abdb116ca14b1baa20-apple-v6, sha256 475fd4366086f3a9c50e55a0c291c11a12afd80acde57c37225130f981f20826 (SHA256SUMS of release run 41643321585). The URL, the checksum, the ghostty-next gitlink, the notices (package-notices.json, vt-link-graph.json) and docs/ghostty-fork.md move together, as #32997 moved them. #70 adds an opt-in macOS mode (ghostty_surface_set_host_present, ghostty_surface_host_frame, ghostty_surface_host_frame_committed). Nothing changes until cmux calls it (next commits, behind motion.frameOwner.terminals). #69 (one compiled link regex per pattern) comes along; #32997 then pins a commit this one already holds (refactor owner decision, STATUS 9071). * fix(next): terminals ready-or-keep: bounds and pixels change in one commit (FRAME OWNER REFACTOR S4, motion.frameOwner.terminals) Films P2: a host-sized Ghostty surface's view took its new bounds at once and the frame drawn at that size came 2-4 refreshes later, so those commits showed the old frame in the new bounds, and two terminals landed 2 refreshes apart. The waiting present (G1) fixed the order at 4-9 ms of main per drag frame. With the Debug switch motion.frameOwner.terminals on (default off, read on every layout): - TerminalHostView places its surface through TerminalSurfaceView.place: a new size goes to Ghostty (set_size, the grid report as before) and the view keeps its size and old frame on the anchor edge of the wanted frame (the host clips it and paints the terminal background beside it). One size waits at a time; the newest goes next. - TerminalFramePresenter (one per window) runs a pass inside the Core Animation commit of each refresh (a 1-point layer the commit displays, after AppKit's layout and display, as CommitProbe does). Each waiting terminal asks ghostty_surface_host_frame(present: true): a drawn frame becomes the layer's contents and the view and its layer take the new bounds in that commit; one not drawn yet keeps both. The main thread never waits. - Requests of one refresh present together for at most 2 refreshes; a request kept for more than 6 passes takes its bounds anyway (counted). - One refresh after a commit that changed a terminal's shown size, the presenter calls ghostty_surface_host_frame_committed, so echo goes off main again; terminals whose bounds changed without a present (first frame, reveal, fallback) are watched until their frame shows. - Springs ask the rest size once (restAt), drags keep the start size until release, a live window resize stays live: unchanged callers, all through place. Switch off: TerminalHostView sets the frame at once, as before, and Ghostty is never put in the embedder mode. * fix(next): S4 presenter: the pass runs before every commit while a request waits, limits count refreshes, a short landing asks again at once The first green run (step 2e4832c0b1d768df79262667) kept every request: the pass ran only when a request or a frame tick armed it, so a frame drawn between ticks waited for the next tick, and on a host with no display link (stall ticks every 100 ms) the last size of a drag never landed. The commit-display layer hook was also unproven at runtime. - The pass is a main run-loop observer just ahead of Core Animation's commit (order 1,999,999, before waiting and on exit), installed while a request waits and removed when none does. It never wakes the run loop; the frame client's ticks bring a turn each refresh. A size asked during AppKit's display cycle (inside that turn's commit) is presented from the next turn. - The group wait (2) and the keep limit (6) count refreshes (frame ticks), not passes, so a busy turn rate cannot trip the fallback early. - A terminal that lands at an older size than the one it wants asks for the newest at once, in the pass. * test(next): a Reduce Motion key resize never holds the main thread for the terminals (FRAME OWNER REFACTOR S4) Refactor owner, films px-keys-single-3k-s1r-reduce (6 refreshes missed per 4 presses) and px-keys-held-2-s1r-reduce: under Reduce Motion each Ctrl-Shift-H/L lands in its own turn, and that turn ran the terminals' whole resize on main. New case in TerminalSurfaceCommitSizeTests: two real terminals side by side in a real layout (Reduce Motion), presses through PaneResize.press. Measures: - the key turn, timed with ContinuousClock from the press to the end of that turn's Core Animation commit (stamped inside CommitBoundary): median of 4 presses <= one 120 Hz refresh (8.33 ms), every press <= two (16.7 ms); - Ghostty's render counters host_display_passes and host_main_draws do not grow (a display pass of a new size waits up to 4 ms for the frame); - every turn keeps ready-or-keep. * test(next): S4 review: a terminal hidden while a size waits keeps its newest size (red); fixtures restore the switch on a failed setup Review of 114c5ef (SHIP, one switch-ON bug): a host layout of a hidden terminal set its frame but left the waiting request, whose landing then put back the older size; a short landing while hidden stayed at the in-between size. New case aTerminalHiddenWhileASizeWaitsKeepsItsNewestSize. Fixtures set the switch only after their checks and restore it and drawsWhenOccluded when setup fails; first-frame waits are bounded by 10 s, not a turn count; the drag case also checks the presenter's stats (presented > 0, no fallback). * fix(next): S4 review fixes: a hidden terminal drops its waiting size, present and bounds under the CA lock, re-watch after a window move Review of 114c5ef: - place() on a hidden, parked or window-less terminal cancels a waiting size before it takes the host's frame, and requestWantedFrame() of a terminal that is not shown takes the wanted frame at once. - The pass presents (ghostty_surface_host_frame present: true) and gives the view its new bounds under [CATransaction lock], Ghostty's call-site rule: no off-main commit of another terminal can ship the new frame with the old bounds in between. A follow-up size goes after the unlock (set_size posts to the renderer thread). - viewDidMoveToWindow re-watches a terminal in the embedder mode, so the new window's presenter reports its commit and echo goes off main again. - TerminalDragTunables doc: the switch is read on every layout. * fix(next): S4 review 2 nits: request cleared before the landing frame change, the tick's present under the CA lock; the key-turn case is median-only over 6 presses, skips missed boundaries, and fails on missing counters or no presents Review of 3bb310c (SHIP): a request the landing's frame change made for the same view was deleted after it (now cleared before); the tick's present for bounds that already fit takes the CA lock (Ghostty's call-site rule). The key-turn case: a hard 16.7 ms bound per press on a shared Debug host could fail without a fault, so it checks the median of 6 presses only and drops samples whose boundary missed the commit; it fails when a terminal has no render counters, and requires the presenter to show each press's frames. * test(next): the hidden-terminal case lays its host out (it set a width with no layout pending, so nothing ran: steps 9b3e8e1e8573d64c9e186171 and ff0ecbe8cc6c06b15083181f failed the same way on test and fix) * fix(next): S4 receipt review nits: a landing that made a request is not also watched; the key-turn case checks both frames per press and no fallback, and requires render counters Receipt review of 39b7b7f (SHIP): the counters' +1 for a missing set cancelled out (now required once in setup); presented >= +6 over 6 presses passed with one terminal presenting (now >= +2 per press and no fallback); landed() could watch a view that the landing's frame change gave a new request (now not). * test(next): S4 key-turn case: each press waits until both terminals landed (at most 2 s), so its presents count for it Receipt review of c4d64b5: 6 main run-loop turns can pass inside one refresh, so the per-press check raced the renderers (and could count a late press's presents for the next). Test only; app source unchanged. * test(next): S4 key-turn case fails and names the terminal and size when a frame still waits after 2 s (never passes quietly) * test(next): S4 main thread keeps turning when motion.frameOwner.terminals goes on and terminals resize under the recorder's per-commit layer reads (red on pin 635c6c12a) TerminalFrameOwnerMainThreadTests runs a real Ghostty surface in a window on screen (a display worker: CMUX_TEST_REQUIRE_GUI or CMUX_CI_DISPLAY_ROUTED). It turns the switch on after the first frame and resizes the terminal three times, each followed by 2 s of output every 4 ms. A layer reads the terminal layer under [CATransaction lock] inside every main commit, as the film recorder's trace sampler does. A sampler thread pings main every 250 ms; no answer in 3 s samples the process and ends it with exit code 70. Red on the pin 635c6c12a (fork #70): step 2d6513308bb18bf5dff69553 (scratch 73ad54e = 71b9d2f + this test) hung. Its sample: main in CA::Transaction::flush > commit > commit_transaction, waiting on an unfair lock; the Metal completion thread in Ghostty's present > CA::Transaction::pop > commit > commit_transaction, waiting on a pthread mutex (it committed while it held the CA lock: fork #70's embedder branch). The fix is fork #71 (next commit moves the pin). * GhosttyNextKit: pin 7d43bb68b (ghostty-next #71: an off-main present never commits while it holds the Core Animation lock) Moves the pin from 635c6c12a to the release branch merge commit 7d43bb68b (release/cmux-9ce9b7d0-e2 = 635c6c12a + fork PR #71): GhosttyNextKit xcframework-7d43bb68b05e1bad433267fe28cd7205041fc4f5-apple-v6, sha256 12f7ac05eec4a0e0a47daef29545cdaba102e4b02ecafcc825d898ff71f4be2f (SHA256SUMS of release run 41705275916; the downloaded zip matches). The URL, the checksum, the ghostty-next gitlink, the notices (package-notices.json, vt-link-graph.json) and docs/ghostty-fork.md move together, as da28dbc moved them. #71 fixes the P0 hang with motion.frameOwner.terminals on (STATUS 9732): in #70's embedder mode the GPU completion thread committed its transaction while it held [CATransaction lock], and main's commit waited for that lock. Every off-main present now does begin, lock, checks, unlock, commit; the default mode already did, so nothing changes with the switch off. Fork evidence: red run 41694823112, green run 41694827381, Codex and Claude SHIP at 3163f4b73. The previous commit's TerminalFrameOwnerMainThreadTests is red on 635c6c12a (step 2d6513308bb18bf5dff69553) and must pass on this pin. * test(next): S4 a new display scale lands with the frame drawn at it, and the landing's commit is reported (red) Codex review of b9bafbd (BLOCK, bug 1): with motion.frameOwner.terminals on, a backing scale change (the window moved between a 1x and a 2x display) set the layer's contentsScale at once, in its own transaction, while the layer still showed the frame drawn at the old scale. For the 1-4 refreshes until Ghostty's frame at the new scale arrived, the old frame showed at the new scale. That frame then landed through Ghostty's main-thread path, which closes the off-main gate (main_pending), and nothing reported its commit, so the terminal's output stopped going to the screen off the main thread until its next resize. aNewDisplayScaleLandsWithTheFrameDrawnAtIt moves a shown terminal to the other scale (1x <-> 2x) and calls viewDidChangeBackingProperties, as AppKit does. It expects: no sampled turn with the layer's bounds in pixels (bounds x contentsScale) and its shown frame at different sizes; the frame came through the presenter; then output goes off the main thread again within 2 s. The test seam: TerminalSurfaceView.backingScaleOverride (nil in the app: AppKit's convertToBacking and window.backingScaleFactor, as before) sets the scale the view converts its points with. No behavior change while it is nil. Red command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests * fix(next): S4 a new display scale lands with the frame drawn at it, through the presenter, and its commit is reported Codex review of b9bafbd (BLOCK, bug 1). With motion.frameOwner.terminals on, a backing scale change now goes through the same ready-or-keep request as a size change: - updateContentScale gives Ghostty the new content scale at once, but the layer keeps the contentsScale of the frame it shows (holdsScale) while that frame is shown (not NONE or HIDDEN). - backingDidChange (viewDidChangeBackingProperties, didChangeScreen) and the reveal in syncWindowState then call requestPresentIfNeeded: a pixel size the shown frame does not have waits for the presenter. Ghostty refuses the new-size frame on its own (bounds x old contentsScale != its size), so the presenter's pass presents it and landRequestedFrame gives the layer the new scale in the same lock span. All terminals of a window that moved request in the same refresh, so they land together. - The landing goes into the presenter's commits, so the next refresh calls ghostty_surface_host_frame_committed and output goes off the main thread again (before, the frame landed through Ghostty's main-thread path, which closes the off-main gate, and nothing reported it). - A size or scale that waits and gives way (switch off, window change, hidden, no window) gives the layer the held scale at once, as before. Also from the same review (risk 2: the lock span calls AppKit's frame setter and the copy-mode cursor sync): setFrameSize no longer syncs the cursor box during a landing; the presenter does it after the lock span (frameDidLand). ghostty_surface_grid_metrics and ghostty_surface_has_selection take the terminal's renderer_state mutex, so they no longer run under the CA lock. The span now holds only ghostty_surface_host_frame(present: true), the frame and layer geometry and the frame-change counter (a leaf lock). With the switch off nothing changes: holdsScale and requestPresentIfNeeded return at once (embedderPresents false), and syncCursor runs in setFrameSize as before (landing is never set). Green command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests * test(next): the S4 hang case fails on a main-thread stall over 3 s that ends, not only on a hang, and prints its phase timings Step df7d7c86adc1556f1f55b5ec (head 30df24f, cmux14) passed after 55 s, though the case needs about 6 s: its sampler saw main not answer for over 3 s and ran /usr/bin/sample, but main came back and the run ended before the sample did, so the case passed and the stack was lost. The same case at b9bafbd (step 0b61836c1fb3b5e4fcbaa678, cmux8) took 6.1 s with no stall. The sampler now: - after the sample, ends the process with exit code 70 only when main is still stopped (a hang, as before); - otherwise writes the sample and the stall's length to stderr, and records the stall with the test's phase (setup, or switch on and resizes); - finish() waits (async) for a sample in progress, so no stall is lost. The case fails on any stall after the switch goes on. A stall during setup (the first surface on a cold worker) is printed, not failed. It also prints the setup time, each resize's time, commit count and longest commit wait, and the presenter's stats. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
This branch was previously 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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
feat-cmux-next tip does not compile its CmuxNextAppTests target (STATUS 8645), from #33120:
The suite is
@MainActor, so itsstatic let casesis main-actor isolated.@Test(arguments: 0..<cases.count)reads it from the macro's nonisolated argument expansion.Fix
static let casesbecomesnonisolated static let cases. The value is a constant array of(SidebarDensity, SidebarMinimalMode); both arepublic nonisolatedSendableCaseIterableenums, so reading it from any isolation is safe. The repo uses the same pattern for@Test(arguments:)tables, for exampleTests/CmuxNextActionsTests/EqualizeAndInterfaceScaleKeysTests.swift:11. No other change.Observed / Not run
cmux-ci compile-check --app --package CmuxNext --recipe-version 2 --ref 45112a704dc8c10b41b196ce9218c0e2e0f9d131) is not yet run; its result follows in a comment. The tests themselves were not run.Changelog
none