P0: Ctrl-` right after Cmd-N shows its terminal in the daemon's pane, not an empty pane (display 0e91fb59 at 88341f28932)
Summary
P0: Ctrl-pressed right after Cmd-N could leave the new terminal tab selected over an empty pane. Two P0 journeys on cmux23 failed this way:r3-after-switch-traced(build cd6a94aa, feat 29efa5800072, STATUS 13438) andr3-e2-reattach` (build 742dc06b, feat 7e6f2ab7f6dc, STATUS 13556). Both waited 20 s for a focused terminal. Their last frames show "New Tab" and the selected terminal tab "~" in the strip over a blank pane. With this change the daemon's pane shows the person's terminal from the turn it takes Cmd-N's stand-in over.
Cause, from the traced journey's input journal: Cmd-N shows a stand-in workspace and pane at once, and the Ctrl-selects and shows its stand-in terminal there. When the daemon's workspace arrives, its pane takes the stand-in over under another layout id, so the window makes it a newPaneController. Nothing moved the stand-in's selection or pending select to that controller, so it started on the daemon's default tab, the New Tab page. The terminal showed there only if the focus expectation landed later. Also, the stand-in's controller had already left the layout, but it still ran the create's end (selectWhenReported). It then showed its own New Tab page, which took that page's view from the daemon's pane (journal seq 134: show pane=pane:6917… from=- pending_tab_21f0…). r3-e2-reattach` recorded no trail, but its frames match the traced failure, so this fix covers both journeys.
The fix makes the hand-over carry the state, with no retry, timer or second show:
DaemonStore.daemonPane(takingOver:)names the daemon pane that took a stand-in over. It uses the same lookup asdaemonTarget.- A pane's waiting select is now the value type
CmuxNextDaemon.PendingTabSelect(it wasPaneController.pendingSelectSurfaceandpendingSelectTab).take(shown:record:)is the rulePaneController.applyused for them.takeOver(_:selected:replacement:)is the hand-over: the stand-in's waiting surface select, else its waiting tab select, else its selected tab (as the daemon's tab in its place, once listed), as one value. A second value would select later what nobody chose then. - The window makes the daemon pane's controller while the stand-in's controller is still mounted, because a new screen takes its panes before the old screen leaves. The new controller calls
takeOverin its init, so its first apply selects and shows what the stand-in showed. A new stand-in pane skips this lookup. - A create made on the stand-in that ends after the hand-over selects in the daemon pane's controller (
successor), not in the stand-in's. The Ctrl-` create and the browser tab opener read that controller's strip. - CmuxNextApp is 1 line below the base (no arch exception).
The split, New Column and New Row stand-ins do not change. Their daemon pane keeps the same layout id and controller (the record is updated in place), and daemonPane(takingOver:) is nil for a daemon handle.
Testing
CmdNThenNewTerminalTabTests.theDaemonsPaneThatTakesCmdNsStandInOverShowsTheTerminalTheStandInShowed fixes the order the trail shows (PendingTabSelectTests covers the value type's rules):
- It stops the daemon, presses Cmd-N, then Ctrl-
. A gate on the workspace'spendingCreateholds the Ctrl-'s create. - It checks that the stand-in shows its stand-in terminal, then resumes the daemon.
- When the daemon's pane mounts, the test requires that pane to select and show that terminal. At that point no daemon terminal and no focus expectation can show it there.
- It opens the gate. The daemon's terminal must then show in the daemon's pane, and the stand-in's controller must show nothing more.
| What | Commit | Fleet job | Result |
|---|---|---|---|
| Red on the base (the test alone) | 6c2716be3ba | efa15d48 (aws-m4pro-1) | red: the daemon's pane took the stand-in over selecting and showing the daemon's New Tab page tab_9392…, not the stand-in terminal pending_tab_53ec… (lines 128 and 130); the suite's other 3 tests passed |
| Red, 10 rounds (never-merge) | 3c1bb21ff4c | 88ed2f4f (aws-m4pro-1) | red in 10 of 10 rounds at lines 128 and 130; in rounds 7-10 also at line 146: the stand-in's controller, out of the layout, showed its New Tab page stand-in again after the hand-over (the trail's seq 134) |
| Green at the head, with the suites of the paths it touches | 88341f28932 | b642d397 (aws-m4pro-7) | green: all 14 suites passed, 88 tests, 0 skipped; the ordered test passed in 0.51 s |
| Green, 10 rounds (never-merge) | 39de72ef940 | 0c1c735e (aws-m4pro-1) | green in 10 of 10 rounds |
| Display run at the exact head | 88341f28932 | 0e91fb59 (display worker cmuxs-Mac-mini-3, console session) | green: all 4 tests of the suite passed, none skipped |
| Compile check, CmuxNext and app (Linux) | red 6c2716be3ba / head 88341f28932 | 916201d1 / 45685209 | passed / passed |
Test targets compile (package-test-lane.sh compile-tests) | red 6c2716be3ba / head 88341f28932 | a661353c / a590e2c1 | passed / passed |
The same build of the failed journey (cd6a94aa) passed r3-after-switch-traced 15 of 15 times on cmux22 and cmux23 with the P0 runner's kit and flags. A 20-round soak of the old, unordered test passed 20 of 20. So a soak does not show this race; the ordered test above does.
Observed / Not run
- Observed: the rows above, and the input journal and pane views of the failed
r3-after-switch-tracedrun (/work/wt/chrome-parity-design/p0-journey-runner/runs/cd6a94aa/). - Not run: the two P0 journeys on a dogfood build of this head. The P0 runner runs them on the next dogfood build after this merges.
- Bench (instant-new-surfaces, Cmd-N and Ctrl-
, 12 runs per phase): a same-host A/B on cmux24-perf in the order base, head, base, head (base 94b22d2354d: d0b1ae51, a6a93662; head 88341f28932: 184618ee and the fourth round). Every key-turn figure of the head is inside the spread of the two base rounds: idle key p95 Cmd-N 16.4 / 15.7 ms base, 16.0 / 15.7 head; Ctrl-13.2 / 12.8 base, 13.1 / 13.1 head; under load Cmd-N 20.4 / 19.2 base, 17.6 / 15.9 head; Ctrl-` 8.4 / 9.1 base, 6.5 / 9.4 head. Visible-frame p95 swings more between two rounds of the same arm (Cmd-N idle 96.2 / 69.6 ms base) than between the arms. Cmd-N's key turn is over the 10 ms budget on the base too; that is the CMD-N FIRST FRAME owner's work (its view reuse follows this PR). The first pair, on two hosts, compares nothing.
Changelog
Fixed: Pressing New Terminal Tab right after New Workspace shows the new terminal in the new workspace, not an empty pane.
Proof
No capture: the change is which tab the daemon pane's controller selects at the hand-over. A still image cannot show the race; the ordered test and the two journeys check it.