test(split): #28436 was the test window, not the key path: type after the hold, and cover typing during it

LA
5d ago
+211-42

Summary

#28436 ("a line typed after a quick double Cmd-D can miss the new pane") was a test rig fault. It was not a product race. SplitOfPendingSplitTests typed while the window's creation hold was still open. The hold kept the keys, and when the focus landed it replayed them through NSApp.sendEvent. NSApp.sendEvent delivers only to the key window, and a test window is never key, so no terminal got a byte. When a person types, the replay goes to the window the person types in, because that window is the key window.

  • The test now waits for the hold to end before it types (as NewTerminalOnPendingSplitTests already does), and it expects the hold to end on focusLanded.

  • A new test, aLineTypedDuringTheHoldRunsInTheSecondPane, follows the person's path: it types at once after the second Cmd-D, during the hold. It expects these:

    • the first split is still pending when the second Cmd-D comes;
    • no split is refused;
    • some keys are held;
    • the line runs in the second pane;
    • the hold ends on focusLanded.

    Its replay runs each key the way the app does (the key router first, then the window) into the test's window, because a test window is never key.

  • Its red form, aReplayThatDropsTheHeldKeysFailsTheCheck, runs the same body with a replay that drops the held keys. That is the shape of #28436: NSApp.sendEvent with no key window delivers the keys nowhere. The test wraps the body in withKnownIssue { ... } matching: { the comment contains "did not run in the second pane" }, with no isIntermittent. The red form is strict for two reasons:

    • When a withKnownIssue block records no issue, Swift Testing records a failure. So if the dropping replay ever lets the line through, this test fails.
    • An issue that does not match (any other failure in the body) still fails the test.

  • The hold's replay is a seam passed at init (PATTERNS P1, cmux-ad item 884). The closure type is HeldKeyReplay. AppDelegate passes { event, _, _ in NSApp.sendEvent(event) } to AppServices, AppServices passes it to KeyRouter, and KeyRouter passes it to CreationInputCoordinator (private let replay). KeyRouter's parameter defaults to the same NSApp.sendEvent call, so its existing test callers do not change. There is no settable property, no static and no global. The app's behavior is the same: each held key still goes through NSApp.sendEvent to the key window, with the same replaying flag and re-entrancy checks.

  • CreationInputCoordinator.HoldEnd moves to CmuxNextFocusUI as CreationHoldEnd, with the same cases. With that move, and with isHolding and hasPendingCreation now one-line expressions with the same results, CmuxNextApp does not grow (SW14).

  • When the typed line does not show, the test now records where it went: every pane's screen, how the hold ended, each terminal view's gate and keyboard, each key's path, and the input journal.

Evidence

Diagnostic run 3, job eae2fd9426eb367df8bf1803 (head 9adddb31f2f, cmux18-perf), had 8 attempts: 3 passed and 5 failed. Each failed attempt reported:

key path (20.13 seconds ago): typing: hold open true, window key false, app NSApplication, second view surface true; e: intercepted true holding true; ... CR: intercepted true holding true
after: hold open false, last end Optional(CmuxNextApp.CreationInputCoordinator.HoldEnd.focusLanded)
pane ... has-keyboard true in-second true screen: sh-3.2$
  • All 24 keys went into the hold. The hold then ended on focusLanded, so the keys were replayed.
  • The second pane had its surface, had the keyboard, and showed only an empty prompt. The other pane showed the same.
  • The input journal had no input attach record for any terminal. The replay reached no window, because window key false.
  • The 3 passing attempts typed after the hold had ended.

Diagnostic run 2, job cd8cdc7cb8cb5af49528587f (head bee8e76bb3a, cmux23-perf), had 5 attempts: 3 passed and 2 failed. The report did not record the key path yet. Its 2 failed attempts show the same state: lastHoldEnd was focusLanded, the second pane had the keyboard, both screens showed only sh-3.2$, and the journal had no input record. Run 1, job 4bb1096c16feda23e4637ccf, had 1 attempt, and it passed.

The person's path: a person's window is the key window. In product code, CmuxApplication.sendEvent passes each key to KeyRouter.interceptKeyDown, and CreationInputCoordinator.capture holds the key. When every creation of the hold has replied and the focus landed with a responder that takes keys, flush(in:) replays the held keys in order through NSApp.sendEvent, which delivers them to the key window. aLineTypedDuringTheHoldRunsInTheSecondPane covers this path. It replays into the test's window, which the rig cannot make key.

Testing

Each count is per attempt. A FLAKY result counts as a failure.

HeadJobWorkeraSecondSplitOnAPendingSplitsPaneMakesASecondPaneaLineTypedDuringTheHoldRunsInTheSecondPane
a87ac4fb182e55adfd211649348f4325973cmux24-perf1 pass (with NewTerminalOnPendingSplitTests 1 pass)-
a87ac4fb182138dbc39987611790c6570c4cmux19-perf12 pass-
a87ac4fb1822416056e7ad4b150b552c022cmux19-perf10 pass-
a1d971b0b5f53258233ceea53bea7212460cmux21-perf12 pass12 pass
a1d971b0b5ffe72b9b00893f7be77443e7ccmux24-perf12 pass12 pass
16a5309a233 (feat-cmux-next merged in)a26206d636f911feb6e69be2cmux24-perf12 pass12 pass
16a5309a233 (feat-cmux-next merged in)7b4d9383bc2d1e7e2c0bb692cmux19-perf12 pass12 pass
435d4652b47 (seam at init)0d24d13186268d3cc6e82155cmux (light)1 pass1 pass
435d4652b47 (seam at init)745851a36dfa78263b76f1fccmux22-perf12 pass12 pass
435d4652b47 (seam at init)8cc154b06ceee4f05c504618cmux23-perf12 pass12 pass

Red form aReplayThatDropsTheHeldKeysFailsTheCheck at 435d4652b47: in each of the 25 attempts (1 + 12 + 12), the check failed on its own assertion, as the red form requires:

━ Test aReplayThatDropsTheHeldKeysFailsTheCheck() recorded a known issue at SplitOfPendingSplitTests.swift:89:29: Issue recorded
↳ the line typed during the hold (24 keys held) did not run in the second pane: wait-for failed: wait-for failed: timeout waiting for pattern
  hold: last end Optional(CmuxNextFocusUI.CreationHoldEnd.focusLanded), holding now false
━ Test aReplayThatDropsTheHeldKeysFailsTheCheck() passed after 20.727 seconds with 1 known issue.

Line 89 is the Issue.record("the line typed during the hold ... did not run in the second pane ...") of the shared body. In each job, the same process ran the red form and the real form with the same keys and the same hold; only the replay was different.

At 4f78e2c08e0, jobs 3a80f141fbe1d98d28777b40 and ecaf648482fc6decab4e2afa did not compile the test target (the new init parameter had no default yet), so no test ran. Those two jobs count as neither a pass nor a fail. Release app compile 8acde2072db8f9a0885cdd56 at 4f78e2c08e0 passed.

Two jobs at e431de4bde5 (04f59f971280f46394aa4429, 27608ab5cdbd03c17ee2ae19) stopped before any test ran. They hit a base red that is not from this PR: "generated file is stale ... Native/OptChat/optchat-chief/inspector/index.html". e20297d7c162 fixed it on feat-cmux-next, and 16a5309a233 merges that fix in. Those two jobs count as neither a pass nor a fail.

The rewritten test passed 96 of 96 attempts. Before this change, with the #28331 fix, it failed 5 of 20 (and 7 of 14 in the diagnostic runs above). The during-hold test passed 73 of 73, and in each attempt it expects that at least one key was typed during the hold.

Each command: cmux-ci run --class light --pool perf-lead --script scripts/ci/package-test-lane.sh --ref <head> --arg=suite --arg=Packages/macOS/CmuxNext --arg=SplitOfPendingSplitTests ... (the suite named once per attempt).

Observed / Not run

Observed: the fleet jobs above, and the prepush gate (no new violations) on each push.

Not run:

  • A GUI run in a real key window (scripts/cmux-next/typeahead-gate.sh ... split). I read the product's replay path, but this PR does not run it in a key window.
  • A red form that drops the keys inside the real NSApp.sendEvent path. The red form drops them in the injected replay instead, which is the same effect that run 3 recorded: the replay reaches no window.

Follow-ups (from the review at 435d4652b47, non-blocking)

  • KeyRouter's heldKeyReplay default (NSApp.sendEvent) is a fallback in production source (P1). The only production construction (AppServices) passes the root's replay. Follow-up: remove the default and give the tests a test-only init. Also make heldKeyReplay private.
  • Two comments still describe the old replay path (AppServices+TestKeyWindow.swift:9, CreationInputCoordinator.swift:18).

Changelog

none

Closes #28436.