Skip to content

Frame owner S4: terminals ready-or-keep, bounds and pixels in one commit (behind motion.frameOwner.terminals, off) - #33956

Merged
lawrencecchen merged 31 commits into
feat-cmux-nextfrom
fix/frame-owner-s4-terminals
Nov 4, 2026
Merged

lawrencecchen merged 31 commits into
feat-cmux-nextfrom
fix/frame-owner-s4-terminals

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

FRAME OWNER REFACTOR stage S4: terminals ready-or-keep. In every Core Animation commit, a terminal's bounds and its pixels change together, or both keep their last size. The films (P2) showed the problem: a host-sized Ghostty surface took its new bounds at once, but 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 earlier fix, a waiting present (G1), cost 4-9 ms of main-thread time per drag frame.

This change is behind the Debug switch motion.frameOwner.terminals (TerminalDragTunables.framesOwnedByCommit). It is off by default, so one build can film old and new.

With the switch on:

  • TerminalHostView places its surface through TerminalSurfaceView.place(_:anchor:). A new size goes to Ghostty with ghostty_surface_set_size, and the grid report works as before. The view keeps its old size and old frame on the anchor edge of the wanted frame; the host clips it and paints the terminal background beside it. Only one size waits at a time, and the newest size goes next.
  • TerminalFramePresenter (one per window) runs a pass just before Core Animation's commit, while a request waits. The pass is a main run-loop observer at order 1,999,999 that never wakes the run loop. For each waiting terminal it calls 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 the same commit.
    • A frame that is not drawn yet keeps both the old bounds and the old frame.
    • The main thread never waits.
  • Requests made in the same refresh form a group and present together for at most 2 refreshes. A request kept for more than 6 refreshes takes its bounds anyway, and this is counted.
  • One refresh after a commit that changed a terminal's shown size, the presenter calls ghostty_surface_host_frame_committed, so the terminal's echo goes to the screen off the main thread again.
  • The callers do not change. Springs ask for the rest size once (restAt). A drag keeps its start size until release. A live window resize stays live.
  • A new display scale (the window moved between a 1x and a 2x display) is the same request: Ghostty gets the new scale and pixel size at once, and the layer keeps the contentsScale of the frame it shows until the presenter shows the frame drawn at the new scale, in the same lock span. The landing's commit is reported like a resize's. (Codex review of b9bafbda1e0.)
  • The lock span holds only the present, the view and layer geometry and leaf work. The copy-mode cursor box (which reads Ghostty under the terminal's lock) follows after the span.

With the switch off, the frame is set at once, as before, and Ghostty is never put in the embedder mode.

Reduce Motion (refactor owner, ROOT-CAUSE.md 6b, STATUS 9359): in a key resize, New Column or Close column, the action's turn lands the layout at once. With the switch on, a terminal whose size changes in that turn does not resize on main: its new size goes to Ghostty, and the surface keeps its old frame (ready-or-keep) until the frame at the new size is drawn. New case: aReduceMotionKeyResizeNeverHoldsTheMainThreadForTheTerminals. It checks the key turn from the press to its commit (median of 6 presses at most 8.33 ms; a sample whose commit boundary was missed is dropped), checks that both terminals' frames come through the presenter on each press with no fallback, checks that no Ghostty display pass or draw runs on main, and checks ready-or-keep in every turn.

Out of scope: creating a NEW terminal in the action's turn (New Column) belongs to TERMINAL CREATE COST, not to S4.

The pin moves in two commits. Each commit moves the URL, the checksum, the ghostty-next gitlink, the notices and docs/ghostty-fork.md together:

The refactor owner decided (STATUS 9071) that #32997 does not hold this pin. #32997 pins dfd07de4a, which this pin already contains.

S2 (#33725) landed as b7aa4eca8d0. The branch merges feat at 0c3f269e809 (with #34198 and the settings-strings autofix, the fixes for the base reds, and S3 #34170).

Testing

Films: the OFF films and the OS smoke ran at c4d64b5 (build ada386be, the old pin). The refactor owner films the switch round, OFF against ON, and runs the typing pairs at this head (15c88481831), because the new pin changes the Ghostty binary of every terminal. The films do not exercise a display scale change (the kit does not move a window between a 1x and a 2x display): aNewDisplayScaleLandsWithTheFrameDrawnAtIt is its evidence.

Film verdict (refactor owner pixel-perfect): build 645ad29d = scratch d16d72b (30df24f + #34198; the same S4 code as 47281ea), switch round ab-s4d-sw-22 on cmux22, 120 Hz 2x, judge v7. 20 of 20 films recorded, motion.frameOwner.terminals OFF then ON in one app, x1 and Reduce Motion, for col drag, row drag, single keys, New Column, close column. ON never hangs: every ON film recorded, and every step after it (the next film, the after-check's real HID Ctrl-C and echo through app, daemon and frame) passed. OFF equals base on the terminal layer (single keys: terminal 27 frames, max 115 px, the same as FEAT DOGFOOD BUILD e0e34bd6; row drag 117 frames at 20 px vs 104 at 20 px). Dropped refreshes OFF/ON per film: 4/3, 1/2, 2/1, 4/6, 1/0. Typing (cmux18, p50/p95/p99/max ms): OFF 3.27/9.07/13.80/20.0 and 3.44/9.87/16.55/41.3; ON 3.30/9.09/12.04/19.0: PASS.

Known with the switch ON, both for the ON PR (the switch stays off here): a row divider drag keeps the old frame longer than OFF lags (content edge off in 156 of 667 frames, max 40 px, vs OFF 117 of 676, max 20 px); and the S3+S4 edge interaction below.

The regression tests are in a new file, TerminalSurfaceCommitSizeTests (CmuxNextAppTests). It uses real MANUAL_MIRROR Ghostty surfaces. After Core Animation's commit, each main run-loop turn samples the layer's bounds in pixels and the IOSurface it shows. The waits use commits and the presenter's own state, never sleeps.

  • aResizeCommitNeverCarriesALayerSizeOtherThanItsIOSurface: 6 drag steps. No turn may end with the layer bounds and the shown frame at different sizes.
  • aReadyFrameIsPresentedInTheCommitThatChangesItsBounds: the first turn with the new bounds shows the frame drawn at that size, and host_fresh_presents grows.
  • aTerminalHiddenWhileASizeWaitsKeepsItsNewestSize: a terminal hidden while a size waits keeps the newest size its host gives it.
  • aNewDisplayScaleLandsWithTheFrameDrawnAtIt: the window moves to the other scale (1x and 2x, through the test seam backingScaleOverride, nil in the app). No turn ends with the layer's bounds x contentsScale and its shown frame at different sizes; the frame comes through the presenter with no fallback; then output goes to the screen off the main thread again within 2 s.
  • aReduceMotionKeyResizeNeverHoldsTheMainThreadForTheTerminals: two terminals, Reduce Motion, 6 presses through PaneResize.press. It checks:
    • the median key turn (from the press to the end of that turn's commit) is at most 8.33 ms;
    • each press presents both terminals' frames, with no fallback;
    • no Ghostty display pass or draw runs on main;
    • every turn is ready-or-keep.

Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests --arg=DividerDragTerminalSteadyTests,SplitTerminalResizeCountTests,TerminalPaneMotionFinalGridTests,KeyboardResizeContentHoldTests,TerminalHiddenResizeTests,TerminalRevealFrameTests

Red runs:

Green: step a906340dc1ad6f98c89d3f80 at 71b9d2fafca, step 72c6951240c5c3f760413a5e at b9bafbda1e0 (the new pin) step f47ab901aef3cf622f57bb6b at 30df24f6b9d and step 13073b52c8029cdcf2c28977 at the head 47281ea9fb2: all 7 suites pass (TerminalSurfaceCommitSizeTests and 6 terminal resize suites).

The display scale case (Codex review of b9bafbda1e0, BLOCK): red at the test commit 1cc1424477e, step 29b9c75c4d2d07c7a5d79ddf: only aNewDisplayScaleLandsWithTheFrameDrawnAtIt fails, with turns of "layer 1200x600 frame 600x300" and no present through the presenter. Green at the fix 30df24f6b9d: step f47ab901aef3cf622f57bb6b.

The deadlock with the switch on (P0): TerminalFrameOwnerMainThreadTests (display workers only: cmux-ci run --display --label gui-session-v1 --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalFrameOwnerMainThreadTests). A real surface in a window on screen; the switch goes on after the first frame; three resizes, each with 2 s of output every 4 ms; a layer reads the terminal layer under [CATransaction lock] in every main commit, as the film recorder does. A sampler pings main every 250 ms; with no answer in 3 s it samples the process and ends it with exit code 70.

  • Red, old pin 635c6c12a: step 2d6513308bb18bf5dff69553 (scratch 73ad54e8a79 = 71b9d2fafca + the test). Main hung in CA::Transaction::flush > commit > commit_transaction; the Metal completion thread hung in Ghostty's present, CA::Transaction::pop > commit > commit_transaction.
  • Green, new pin: step 0b61836c1fb3b5e4fcbaa678 at b9bafbda1e0 (6.1 s, no stall) and step df7d7c86adc1556f1f55b5ec at 30df24f6b9d. That second run took 55 s and its sampler fired once: main stopped for more than 3 s, then came back. The old test passed it quietly, so 4b529209769 makes the case fail on any main stall over 3 s after the switch goes on, and print its phase timings. The stricter case at 30df24f6b9d passed in 6.1 s with no stall (step 0fa5064b4395cd050c0b504f: each resize 2.0 s with 33-36k commits, longest commit wait 24.5 ms, presenter fallbacks 0).
  • At the head 47281ea9fb2: step ea99af46f70d0b028e2b1780.

Observed / Not run

Changelog

none

…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).
…(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)
… 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).
…er 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
… 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.
…ime 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
… (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.
…resents 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).
…ommit (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.
…quest 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.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 15c8848: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr33956 -f ref=fix/frame-owner-s4-terminals -f platform=macos -f backend_mode=local

Not verified: macOS jobs did not run: compile and app tests were skipped on f911a4dc7c; ci-status is green without them.

CI passes on f911a4dc7c (run 41749730768 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

web and size checks: green

Default changes

This pull request changes 3 defaults at f911a4dc7c0d (what everyone gets without configuring anything):

  • tunable-source.CmuxNextDesign/Tunables/TerminalDragTunables.swift:all: "d088190b683e5d268190fa20347da16af582879e5f5562ecdd3b41c095227d21" -> "cab3d3153edd27ef2c43eb3abd1a3819800e7847845fef5e2100863692ffbbd4"
  • tunable-source.CmuxNextDesign/Tunables/TerminalDragTunables.swift:framesOwnedByCommit: (none) -> "cec46348c4368c8f87f121ad539c767297f5b6c6c8b1c8984e0aa7dbd83f3efb"
  • tunable.motion.frameOwner.terminals: (none) -> false

Signed off on this commit by review (codex), carried from 47281ea9fb23 across base merges.

Merge receipt: merged as e5ccc759105e with these defaults changed, signed off by review (codex), carried from 47281ea9fb23 across base merges.

Regenerate the snapshot with python3 scripts/ci/defaults_guard.py snapshot.

Shipped in:

@blacksmith-sh

This comment has been minimized.

…r 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.
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (codex): SHIP at 30df24f

Codex review (codex exec -s read-only, code inline: the scale fix diff b9bafbd..30df24f, the whole PR diff, TerminalSurfaceView.swift, TerminalSurfaceView+FrameOwner.swift and TerminalFramePresenter.swift at the head). It re-checks the BLOCK at b9bafbd (bug 1: a display scale change). Findings:

No blocking bug found in the supplied diff.

  1. Bug 1 fixed — TerminalSurfaceView.swift:371, TerminalSurfaceView+FrameOwner.swift:125,145,81. Creation calls updateContentScale while unshown, so it cannot hold a scale. Both other callers—backingDidChange and syncWindowState—follow scale/size updates with requestPresentIfNeeded. READY/PENDING/HELD creates a presenter request; an existing request carries the scale. Landing applies it under the same lock as presentation. NONE/HIDDEN/SHOWN applies it immediately. Hidden placement, release, and fallback also drain it. I find no held-scale-without-request freeze path.

    Risk, accepted exception — TerminalFramePresenter.swift:211, TerminalSurfaceView+FrameOwner.swift:112. Fallback and ownership release can still commit an old frame with the new scale, just as they can with new bounds. Thus “no mismatched commit ever” remains stronger than the accepted design. Fix: qualify that invariant in documentation.

  2. Switch OFF: behavior preserved — TerminalSurfaceView+FrameOwner.swift:126,146, TerminalSurfaceView.swift:362. With embedderPresents == false, holdsScale returns false and requestPresentIfNeeded returns immediately. The ordinary scale transaction remains. landing stays false, so setFrameSize retains its previous size update and cursor synchronization. The Ghostty pin itself changes shared implementation in both modes; its binary internals cannot be independently assessed from this diff.

  3. Risk: remaining transitive lock work — TerminalFramePresenter.swift:191, TerminalSurfaceView+FrameOwner.swift:87, TerminalSurfaceView.swift:360. The explicit span now contains presentation, scale/geometry setters, dictionary bookkeeping, and frameChanges.bump(). Both explicit routes to Ghostty size/grid reads and cursor synchronization are suppressed during landing; cursor synchronization runs afterward at TerminalFramePresenter.swift:230. No explicit commit/flush/draw remains. AppKit’s setter and frameChanges.bump() implementations are not supplied, so their transitive behavior cannot be certified here. Fix: keep those paths leaf-only and audit their implementations.

  4. Reveal handling is correct — TerminalSurfaceView.swift:279,286, TerminalSurfaceView+FrameOwner.swift:145. A hidden size is reported before the new presentation check. That check requests presentation even without an inFlight point-size change, which is necessary when the bounds already changed while hidden. A held scale follows the same request, or is applied immediately when nothing needs replacing. A HIDDEN result gives way consistently with the accepted reveal behavior.

  5. Risk: test timing sensitivity, not incorrect synchronization — TerminalSurfaceCommitSizeTests.swift:49,69,74. The test uses commit boundaries, not synchronization sleeps, and checks both pixel consistency and presenter participation. At the red commit, immediate scale assignment causes the intended mismatch when rendering lags; even a fast renderer cannot satisfy presenter participation through the missing scale-request path. However, zero fallback and the two-second off-main deadline depend on renderer scheduling. Fix: use a controllable frame-readiness seam if strict repeatability is required.

  6. Whole supplied PR: no additional actionable bug identified. This is static review; tests were not run.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Non-green checks at 30df24f6b9d, classified:

…at 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.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (codex): SHIP at 15c8848

Codex review (codex exec -s read-only, code inline: the test diff 30df24f..4b52920, CmuxNextCompat Mutex, TerminalHostView+PaneMotion.swift and PaneMotionHold.swift (S3, from the feat merge), TerminalHostView.swift and TerminalSurfaceView+FrameOwner.swift at the head, and the whole PR diff). Since the SHIP at 30df24f, the head adds the stricter hang test and a feat merge. Findings:

No blocking bug established from the supplied diff. Review was static; no commands or tests run.

  1. Risk — TerminalFrameOwnerMainThreadTests.swift:265–279: sampler attribution is imperfect. A semaphore timeout measures an unanswered queued ping, not necessarily continuous main-thread execution blockage. Worker scheduling delays, debugger pauses, or unrelated main-thread work can fail this test. A response arriving just after the timeout can even produce a recorded duration below three seconds. Check answeredAt against the deadline before recording a stall.

    Exit 70 means the semaphore remained unsignalled at the final check; it cannot strictly prove main is still stopped. Main can resume immediately afterward, or be preempted between recording answeredAt and signalling. This is a watchdog limitation, not evidence of a product deadlock.

    Otherwise, for this single-start, single-finish() usage, the sampler looks concurrency-correct: the noncopyable mutex stays stored in the class, shared state is locked, and no nested locking occurs. finish() suspends main rather than blocking it. Registration and watcher termination serialize through the mutex; the continuation resumes exactly once, outside the lock. Concurrent calls to finish() would overwrite the continuation, but this test makes only one call.

  2. Risk — TerminalSurfaceView+FrameOwner.swift:191–194; TerminalHostView.swift:397: transient S3 edge differences. Yes, the kept surface can occupy a different origin from producerFrame(oldSize, in: shownArea). For a trailing hold, S4 retains the wanted frame’s leading edge, rather than recalculating the old surface’s trailing alignment. For hold.bottom == true with no bottom surfaceHold anchor, S4 defaults to top alignment and temporarily shifts the old surface vertically.

    Once the requested size lands, anchoring equal-sized rectangles yields exactly S3’s wanted frame. Origin-only changes also apply immediately when no size is pending. Thus this is the specified S4 keep policy interacting with S3, suitable for film judgment rather than an established blocker. If retaining S3’s stationary edges during the wait is required, carry both horizontal and vertical hold edges into S4.

    S4’s supplied tests check sizes, pixels and progress, not these origins. S3 tests running with S4’s default-OFF switch should retain their behavior; no S3 test bodies were supplied to assess switch-ON assertions.

  3. Scope limitation: no additional concrete bug established in the supplied whole-PR diff. The Ghostty binary/submodule implementation is not included, so its locking guarantees cannot be independently verified here.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (codex): SHIP at 47281ea

Codex review (codex exec -s read-only, code inline). The only commit since the SHIP at 15c8848 is a merge of feat, which brings the base fixes for the tunnel and settings-strings reds. The PR's own diff against its merge-base is line-for-line the same (0 differing +/- lines). Findings:

No blocking findings in the supplied evidence.

  1. The PR diff matches S4: motion.frameOwner.terminals defaults to false; host placement routes through place; the presenter coordinates frames, bounds and display scale; tests cover commit sizing and main-thread hangs. The submodule, binary URL, notices and link graph consistently reference 7d43bb68b05e1bad433267fe28cd7205041fc4f5. No unrelated feat changes appear in the PR-owned diff.

  2. None of the 156 files in the merge stat is one of the named S4 files or pin files. The stat therefore shows no direct merge modification to those files. Indirect behavioral interactions cannot be established from a stat alone, but nothing supplied identifies an S4 conflict.

Given the supplied zero-difference comparison against the previously approved PR diff and the merge-only first-parent history, the prior verdict carries forward. This assessment uses only the supplied material; no commands or tests were run.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

The three failed or cancelled steps at 15c88481831, classified:

  • 7 suites, step f723ed7aa94b6c3f75502849: a base red. "error: the cmux-next web bundles did not build", because 63 settings strings were missing (settings.cloud.tunnel.enabled, settings.group.cloudTunnel, ...). Feat fixed this (ca66a4c92b9, base autofix).
  • Hang test, step 178668b82cd238d2198bcc40: the same base red, the same web bundle error. The test did not run.
  • OFF bench, step 9aff9b25bc7bf6e166012780: infra. The step was cancelled on cmux10 while it compiled Rust crates (terminated by signal), and its summary upload failed. It never reached the app or the bench.

Action: I merged feat (47281ea9fb2; the PR's own diff did not change) and started the three steps again at that head: 13073b52c8029cdcf2c28977 (7 suites), ea99af46f70d0b028e2b1780 (hang test) and ca23efeac937ddfa93951d4e (OFF bench).

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Non-green checks at 47281ea9fb2, classified. None comes from this PR, which touches no file named below:

  • cmux-next checks: a base finding. The red check is "Main-thread concurrency" at Sources/CmuxNextPagesTestSupport/FakeChromiumHosting.swift:44 (an unbounded AsyncStream). The step "Pushed commits add no violations" passed.
  • cmux-next swift test and cmux-next fast tier: the red tests are OmnibarFocusLayoutShiftTests:78, NativeTooltipGuardTests:37 and HomeSidebarUnreadDotTests:55. These are omnibar, tooltip and Home sidebar tests, not terminal tests. The fast tier was cancelled. All S4 suites pass on the fleet at this head: step 13073b52c8029cdcf2c28977 (7 suites) and step ea99af46f70d0b028e2b1780 (the display hang test: 6.1 s, no stall).
  • cmux-next daemon tests: ClassicUpgradeSimulationTests:184, classic migration, unrelated.
  • CmuxMobileKit tests: CloudChatsLiveTests (relay hub readiness), the same flake as at 30df24f6b9d. The mobile kit does not build Packages/macOS/CmuxNext.
    The merge-freeze check passes (exit 0). The Codex receipt is at this head.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026

This branch was successfully deployed

2 inactive (outdated) deployments
ci-routing-agent — f911a4dc Deployed Nov 4, 2026 by cmux-bot[bot] via cmux-tui tree preflight #108333
artifacts — 47281ea9 Deployed Nov 3, 2026 by lawrencecchen via publish tree-addressed cmux-tui #107960
sentry-dsyms — 47281ea9 Deployed Nov 3, 2026 by lawrencecchen via upload cmux-tui dSYMs to Sentry #107960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant