Repository navigation
Keyboard pane resize: each terminal resizes once per press, with no old-layout frame (D1, regression of #32635) - #33607
Conversation
…nal 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>
…o (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>
…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>
…wner-s3a-one-sigwinch
…ond 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>
…ain-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>
…e 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>
Bugbot needs on-demand usage enabledBugbot 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. |
|
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:
|
|
Found 2 test failures on Blacksmith runners: Failures
|
|
CI reds on head 519815d: all are base findings or wrappers of base findings. None is in a file this PR changes.
The Swift evidence for this change is the fleet runs in the description: red 3789fb93581a3a6a26245771, green 47291d84792f40ffeeb1d851, and GUI proof d12081b37c8a7f86327df30c (all 11 suites passed). |
…en-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>
Bugbot needs on-demand usage enabledBugbot 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. |
…g 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>
|
Review (claude-review-subagent): SHIP at 92dd886 Independent review subagent (correctness first, read-only, per skills/cmux-review/SKILL.md). Its first verdict at 519815d was FIX: Findings at this head:
What the review checked: the release order through PaneContentView, RestingPlacement, TerminalSurfaceView, TerminalHostView, TerminalGridPolicy and CEFHostView; hidden and parked screens (the key-hold flag is cleared by every hide or park path, so the takeover loop cannot run there); the pane the change removes (it lays out once, gets no surface resize and sends no grid); zoom takeovers; re-entrancy; and that the new p2 check fails without the fix for the stated reason and is not flaky. Not checked by the reviewer: nothing was built or run (repo rule); the fleet runs are in the description. |
…wner-s1-strip-tick Conflict in PaneHostView.endKeyResizeHold with S3a (#33607, D1): feat's form is kept (one-line guard, layOutLetGoContent); S1 keeps its one change there, heldContentSize = heldContentSize ?? content.frame.size: with only the content area held (motion.frameOwner.tabStrip) the content view is already at the pane's size, so the size a structural takeover keeps holding is the held one. S3a's release order composes with S1: layOutLetGoContent runs layoutClip, which lets the held area go (syncHeldArea), then lays the subtree out, then the content hears the landing.
|
Review (codex): SHIP at 1834e2d
Review after the merge, by Codex (model gpt-6.1-sol), posted by the FRAME OWNER REFACTOR owner (agent pixel-perfect). feat-cmux-next's rule since #33263 wants a different-model review for a behavior change; this PR landed with Claude reviews only, so this review is after the fact. The reviewer had no tools: the prompt held the source diff, PaneHostView.swift and TerminalSurfaceView+PaneMotion.swift at this commit, and the two test files. Finding 5's gap (tests do not observe rendered frames) is covered by the films in the PR body: 120 Hz scan-line films of the landing (no old-layout frame at the Ctrl-Shift-H landing; SIGWINCH 24 -> 8 for 8 presses). The nits do not need a follow-up PR; finding 2 (a pane detached mid spring) is noted for S3, which owns the hold policy. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
A keyboard pane resize (Ctrl-Shift-H/J/K/L) sent each terminal three PTY resizes per press (final, start, final) instead of one, and the terminal showed its old layout for one frame at the landing. This is D1 of the FRAME OWNER REFACTOR (stage S3a), a regression of #32635 (RESIZE KEYS K3). Film proof (ROOT-CAUSE.md P4, STATUS 8594): the
sigwinchTUI counts 24 SIGWINCH for 8 presses on builds with #32635 (52788515 = feat 5851715, 1d8e759e = feat 5fe33a3) and 8 on builds without it (8b6dcff3 = feat 507438d, 68357686). Film rk-browser-agent (build 52788515) frame 269 shows the program's 35-column output between 38-column frames.Cause: K3 holds each pane's content at its start size during the key's spring and releases it after the landing frame's commit (
PaneHostView.releaseHeldContent). The release set only the pane content's frame (layoutClip).PaneContentViewframes its content host inlayout(), so the content host and the terminal host in it kept the start size until the next layout pass. Then the content heard that the motion settled:TerminalSurfaceView.endPaneMotion->TerminalHostView.releaseSurfaceHoldsized the surface from that stale host (start size, start grid), and the next layout pass sent the final grid again. A Chromium page heard the landing at its stale frame the same way.Fix: content that a hold lets go lays its whole subtree out (
layOutLetGoContent:layoutClip, thenlayoutSubtreeIfNeededwhen the host has a real size in a window) before it hears anything, so every view in the content has its frame first. Both letting-go paths use it:releaseHeldContent, beforenotifyPaneMotionSettled: the K3 release after the landing commit,endKeyResizeHoldfor a pane at rest, and a spring's held content at its landing frame (agent panes, the provider'sholdsContentInMotion). It is the same one content layout as before, in the same after-commit pass, moved before the notification.endKeyResizeHoldfor a pane still on its key spring when a split, close or zoom takes the screen over (found in review): the structural change'sannounceRestused to measure the terminal from its held host, so the terminal sent a wrong "final" grid at the change's start and the right grid at its landing.The sidebar slide landing (
WindowRootView.wireContentAreaMotion) already laid out before it notified; spring landings without a hold and drag releases were already in that order. No runtime switch: this is a regression fix.PaneHostView.swiftis one line shorter than on the base.Testing
New tests, each in a new file, committed before the fix:
KeyboardResizeOneGridReportTests: the app'sPaneContentViewwith a realTerminalSessionin each pane. One Ctrl-Shift-H/L/K/J press, the spring stepped with a manual 120 Hz tick, the after-commit release, one more layout pass. Each terminal must send the final grid once (at the press) and nothing at the landing, and its surface must never take its start size again. A second test coversendKeyResizeHold: a second press and, 6 frames into it, a close arrive before the first press's landing commit. p1 (at rest) must send nothing more; p2 (taken over on its spring) must send one grid for the close, its final one.PaneHeldContentLandingTests: a host with an injected content hold (holdsContentInMotion: { true }) whose landing frame only moves it.Red at 94db280 (tests, no fix): step
3789fb93581a3a6a26245771(aws-m4pro-5) FAILED as expected, in all three release paths. For example, p1 on Ctrl-Shift-H: 66x44 at the press, then 74x44 (start) and 66x44 again at the landing; surface widths 540 -> 600 -> 540 pt. (Earlier red of the key test alone:47b106dfc7b2a3d5a929a39eat a6b34db.)Red at f1f7ae0 (the takeover check, first fix only): step
bb3b972222294053e40f7a0f(aws-m4pro-6) FAILED as expected: p2 asked for 73x43 (wrong) and then 66x44; every other test passed.Green at 519815d (first fix): step
47291d84792f40ffeeb1d851(aws-m4pro-1), 40 suites: the new tests, every keyboard-resize suite, the terminal grid suites (TerminalPaneMotionFinalGrid, SplitTerminalResizeCount, PaneZoomTerminalResize, EarlyTerminalGrid, DividerDragTerminalResizeCount), and every suite that steps pane springs (split, close, zoom, burst, column and row close, catchers, sidebar slide, chat dock, hidden screens, pixel grid). 39 of 40 passed.ParkedPageCatcherPanelsTestsfailed only because it needs a screen: on that headless worker theShellWindowframe was infinite (NSWindowinit threw!CGRectIsNull(frame), andcatcherAreaswas empty). All its earlier passes were on display workers. It runs in the GUI proof step below.GUI proof at 519815d, on a display worker: step
d12081b37c8a7f86327df30c(--display --label gui-session-v1,CMUX_TEST_REQUIRE_GUI=1; the new tests, the keyboard-resize and terminal grid suites, andParkedPageCatcherPanelsTests). Result: PASSED (state done, cmux10s): all 11 suites passed, 0 failed, 0 skipped.At this head (92dd886):
1d2e176b89e75d77bcf9c132passed.fa91838a7e6a9ac602ec10f8(aws-m4pro-8), the same 40 suites: 39 passed, including both new test files and the takeover check.ParkedPageCatcherPanelsTestsfailed again on a headless worker: its 4 tests passed, then the suite's window-leak check found 7DividerMousePanelwindows left open. The suite needs a screen. The change from 519815d to this head runs only when a key hold meets a structural change, and this suite has no key hold.c585314b120e084bd56c8d2f(cmux14,gui-session-v1,CMUX_TEST_REQUIRE_GUI=1, the same 11 suites includingParkedPageCatcherPanelsTests). Result: PASSED (state done): all 11 suites passed, 0 failed, 0 skipped.Review (claude-review-subagent): SHIP at 92dd886cc1399c1827bf48ecc26a7638f962f15a(PR comment).Commands:
Film proof, after (refactor owner pixel-perfect, STATUS 8878): the PR build 6a00a4e75d66815521a28cd3 (head 519815d) against FEAT DOGFOOD BUILD 4d9e624a, both on cmux24, 120 Hz 2x, kit cp/pixel-perfect-kit ae6563d.
resize-keyboard, 8 presses: SIGWINCH 24 on the base, 8 on the PR (one per press).px-keys-single-3k-s1r-s1at the Ctrl-Shift-H landing: the base shows the wrong program layout for 2 frames (267, 268); the PR goes from the spring straight to the right layout (268)./work/tmp/resize-videos/runs/ab-s3a-base-77e9a45-20261103T105654Z-cmux24and/work/tmp/resize-videos/runs/ab-s3a-pr-519815d-20261103T105944Z-cmux24. Gallery: https://cmux-orchestrator-1.tail137216.ts.net/resize-videos/ab-s3a-base-77e9a45-120x2/ and https://cmux-orchestrator-1.tail137216.ts.net/resize-videos/ab-s3a-pr-519815d-120x2/bb3b972222294053e40f7a0f, greenfa91838a7e6a9ac602ec10f8at this head.TYPING UNDER LOAD (refactor owner, typing-load-l6, P0 runner flags, kit ed8b0993f23, cmux24): the base FEAT DOGFOOD BUILD 4d9e624a passed (call_ms p50 29.65 / 30.14), and the PR build 6a00a4e7 (head 519815d) passed (call_ms p50 28.34 / 27.93). Runs:
/work/tmp/pixel-perfect/typing-runs/tul-base-77e9a45-20261103T112115Z-cmux24and/work/tmp/pixel-perfect/typing-runs/tul-s3a-519815d-20261103T113052Z-cmux24.Compile-checks (Linux pool, CmuxNext with tests, app, cmux-tui):
66efcb87f902bf4b674a1449at 94db280 andd9a9687b7a94966693ba229bat 519815d passed.Observed / Not run
Observed: the red and green steps above, and both compile-checks.
scripts/cmux-next/prepush-gate.pyandscripts/cmux-next/check-motion.shpass.Not run:
/work/wt/chrome-parity-design/mac-lane-allow.txtthat I added without the lead's OK. The refactor owner asked the lead (STATUS 8699). The line waits for the lead's answer.RowBadgeSpinnerDensityTests). For the fleet runs I cherry-picked base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (from #33120) #33377's one-line fix (a6b34db). base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (from #33120) #33377 landed at 08:57Z, and this branch merged feat 960d3eb after it, so that commit is a no-op and the PR diff has only this change.Area owners told in STATUS (8597): PL-motion-core (
PaneHostView.swift) and PL-ghostty (the terminal surface hold).Changelog
Fixed: a keyboard pane resize (Ctrl-Shift-H/J/K/L) resizes each terminal once, not three times, and never shows the old layout for a frame
Proof
No capture: the change is in when pane content lays out after a key resize lands; the GUI proof step and the refactor owner's 120 Hz film rounds are the evidence.