Repository navigation
fix(next): keys held for several Cmd-W checks reach the terminal in typing order (#32291 follow-up) - #32409
Merged
Conversation
…t not take the held keys Red on feat: Cmd-W on an idle terminal hides it and its read is held; keys are typed; the close button of another terminal (running cat) is pressed; more keys are typed. After the release, the running terminal asks and the person cancels: the keys typed during the read must reach the focused neighbour and never the cancelled close's terminal (review Low of #31875).
… for the focused terminal's close While a hidden Cmd-W read was open, CreationInputBuffer.isClosing(tab:) was true for every tab, so beginClose opened a key lane for any other terminal's close (its close button). That close took every key typed after it, and when the first close ended, settleClose moved its keys to that newest close. Its program asked; the person cancelled; the keys were typed into that terminal. A close opens a lane only when it takes the focused terminal, or, while that terminal's close still reads, the tab its pane selects now (the quick second Cmd-W). The keys of the first close then go to the focused terminal (or wait for an open question's answer, then go there).
…minal Pins the stand-in exclusion of #31875 (review Low): with unanswered set, a stand-in that is asked counts as a running program and the close asks "Still running: <title>" for a terminal that runs nothing.
…o on after its answer The hidden close of the focused terminal ended just before another close's question showed: its keys were settled for the focused terminal, but the hold's flush ran a turn later, with the question up, and the key router gave every replayed key to the question (answersBlockingDialog): the keys reached no terminal, and a held Return would press the question's Close. While a close's question is up in the window (parkClose), the hold keeps its keys, also at its time limit; the answer (endClose) lets them go on, each to the terminal it was typed for.
…it for every open question Review of #32291 (High): flush kept the hold while the question showed, so capture took the keys typed at it (Return, Escape): only a click answered. Now capture passes every key on while a close's question is up in the window, and the held keys still wait. asking counts the open questions (Low: a second question in the same window got the keys at the first one's answer); the keys go on once none is up.
…keeps its keys for the focused terminal Red at 5e62cc0 (review of #32291, Medium): with the running terminal's read ending first, the idle close's keys joined that question's keys, and Cancel typed them into the running terminal. DebugCloseReadHold.releaseNewest (DEBUG and bench builds) lets the test order the two answers.
…l that stays Keys of other closes that ended while the question asked go to the focused terminal, as on Close (review of #32291, Medium). A closed window drops its open-question count (review Low).
…c7b66) The reverse-order test passes without it (step 2615afe3093a at 486ef33): after the first fix, a close of a terminal that does not have the focus opens no lane, so its parked close names no terminal, and Cancel sends every waiting key to the focused terminal. The split changed no observable case, so endClose is again as at 5e62cc0. The window-close cleanup of the open-question count stays.
…ys-2/held-keys-other-close # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift
… typing order Red on #32291's head (review Low there): Cmd-W on an idle terminal, keys, a quick second Cmd-W on a running neighbour, more keys. The neighbour's question shows before the first close's read ends; that close ends while the question asks, and its keys were put after the question's own keys, so after Cancel a typed "ab" arrived "ba".
… last answer Root cause: a close's question took its keys out of the window's hold (parkClose), and a close that ended while the question asked appended its keys after them; Cancel then typed the joined list at once. Keys typed in order "ab" (first close) then "cd" (second) arrived "cdab". Now the window's hold buffer is the one ordered queue: a question's keys stay in it (CreationInputBuffer.answering), a close that ends meanwhile hands its keys to the newest open close (reading or asking), and an answer only names their terminal (settleClose stays: the terminal asked about). The hold lets them go after the last open question's answer, in typing order. ParkedClose now carries only the ticket; heldKeys(_:for:) had no other caller and is gone.
…ys-2/held-key-order # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift # Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CmdWIdleTerminalClosesInItsTurnTests.swift
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 2, 2026 15:03 — 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 2, 2026 15:03 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (claude-subagent): SHIP at 83a2ccf Correctness review of the three changed files, with the callers TabCloseGuard.close, CloseUndoToasts.decideFirst, and the coordinator's capture, flush, resolve, runAgain and take(focused:). I only read the code. I did not run anything on this machine.
Low (do not block):
|
…till reads, as before Review of #32409 (Low): with only unanswered closes handing keys on, a Close answer while another close still read (a pane it would empty, so not hidden, with the focus on its tab) gave the keys a lane to that closing tab, and they went with it. The old path handed them to the reading close; this keeps that. Cancel never hands them on.
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 2, 2026 15:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 2, 2026 15:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 2, 2026 15:14 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (claude-subagent): SHIP at be3d581 I reviewed the head be3d581 (one commit after 83a2ccf, CreationInputBuffer.settleClose only) by reading the code. I did not build or run anything on this machine.
|
lawrencecchen
added
the
ready-to-land
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…sions, dialogs, page-first chords), Rust only (#34052) * docs(focus-core): the agent cursor visibility rules are called through the typed C ABI The module comment still said the app asks over the JSON operation. Since #33313 the Swift resolver calls cmux_agent_cursor_visibility; the JSON operation stays and its FFI test replays the vectors. * test(input-router): the router's scenarios and model-based fuzz, red on today's two key rules (R-INPUT S1a) The one input router (handoff/rewrites/R-INPUT/DESIGN.md, stage S1a) in cmux-focus-core::input, with its tests: a scenario for each key defect class R-INPUT ends (P0 1193, REOPEN-F4/F5, #32633, close-then-type, INVERSE-KEY, a click before the first paint, the cross-window key, keys for Home after a new terminal), and a seeded model-based fuzz against a plain reference model, with seven mutants it must catch. This commit is the red: its router keeps TODAY's two rules of the app's key paths, so the tests fail here: - AppKit's dispatch decides where a key that runs no binding goes (the first responder of the key window), as the key window's first responder does now; - a held key runs again through the router when it leaves its hold, and goes where the focus is then (the CreationInputCoordinator / NewTabInputCoordinator replay through NSApplication.sendEvent). The next commit removes both: a key's sink is decided in the key's own call from the window's focus, and a waiting key leaves its lane only to that sink. The topology's Tab gets the host terminal id (`terminal`), so a creation's stand-in tab and the daemon's tab are one terminal sink. The C topology carries none yet (the FFI decode sets None; the app sends it from S3). Run: cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --os linux --arg test --arg key_router:: --ref <this sha> * fix(input-router): a key's sink is fixed in the key's own call; a waiting key leaves its lane only to that sink (R-INPUT S1a) Removes today's two rules from the router (see the red commit): - the sink of a key that runs no binding comes from the window's focus at key time: the tab the focus is on its way to (a creation's), else the focused target. AppKit's dispatch only chooses HOW the key reaches that sink (its responder when it reaches it, else the sink's own path), never which sink; - a key that waits stays in its sink's lane until that sink can take it, and leaves only to it, in typing order; when the sink ends the key is dropped with the reason. No re-aim, no replay through the router, no timer. Rust only, no caller yet: the app links it from stage S3 (shadow), and the behavior stages flip one runtime switch each (DESIGN.md section 5). * fix(input-router): the router's send-or-queue step is route_to, not to_sink (clippy wrong_self_convention) A to_* method that takes &mut self breaks clippy's naming rule (green step 759965538c90: fmt and the 25 tests passed, clippy -D warnings failed on this name only). A pure rename; no behavior change. * test(input-router): key sessions, dialogs, page-first chords and the Escape rule, red on today's rules (R-INPUT S1b) Stage S1b of the one input router (handoff/rewrites/R-INPUT/DESIGN.md sections 4.4, 4.7 and 4.8), Rust only, no caller yet: - key sessions (link hints the person started, window hints, the shortcut recorder): at most one runs (I5), and a key it takes reaches no sink (I4); - blocking dialogs (a window's or one tab's): a key typed under a dialog is the dialog's, a key typed before it waits for the answer with its own target; - page-first chords: a chord a page may own goes to the page first, and its action runs once, by the key's own id, only when the page leaves it unhandled; - Escape in a field the person moved into from content gives the keyboard back to that content (#33049). The tests: 20 scenarios (#32537 review rounds, #31849, #32291/#32409, #32849, #33049, with their STATUS items), the reference model and the fuzz extended with sessions, dialogs and page reports, and four more mutants the fuzz must catch (a hint letter to the page, two sessions, an action run twice, a key under a dialog to its target). This commit is the red: the router keeps TODAY's rules (no key session in the router, a dialog gets keys only through AppKit, a bound chord runs at once, Escape stays in the field), so the tests fail here. The next commit adds the rules. * fix(input-router): sessions take their keys, dialogs block their targets, page-first chords wait for the page's report, Escape returns from a field (R-INPUT S1b) Adds the four rules to the router's one decision per key (see the red commit): - a running key session takes its window's hint keys (letters, Backspace, Escape; the recorder: every key) in the key's own call; any other key, a key of another window, a focus move off its target, its window closing or a newer session ends it, and that key then routes as usual. At most one runs; link hints start only on the page the window's keys go to; - a dialog that blocks a key's target takes the key (a bound chord still runs), and a target a dialog blocks takes no key: keys typed before the dialog wait for the answer and go on in order, or are dropped when the answer closes their tab; - a chord a page may own (Find, a page-editing chord, a content action) goes to the focused page first when the page takes keys now; its action runs once, aimed at the key's tab, when the page reports that key unhandled. A report of a newer key forgets the older offers (a page reports in key order); - a bare Escape in a sidebar or text field the person moved into from content gives the keyboard back to that pane, unless the field uses Escape itself. Rust only, no caller yet: the app links it from stage S3 (shadow), and S6/S7 flip input.router.pageKeys and input.router.overlays (DESIGN.md section 5). * test(input-router): sink lifetimes, creation replies, actions that wait, the key's own target for its binding (R-INPUT S1 review), red Scenarios for the findings of the first independent review of PR #34052 and for the fuzz failure of the S1b green step (fleet step 1803a1b30c2e, seed 39, I7: a key reached Host("T2") after the ledger saw T2 end): - a terminal that ended takes no key while its tab still shows, and takes keys again only when a window shows it after none did (seed 39); - Reopen Closed brings a tab back under its old id: the new page takes keys after its own first paint; a creation that names an ended tab again makes it a new sink (finding 1); - a creation that fails after its stand-in showed still ends its terminal: waiting and later keys are dropped with the reason (finding 2); - a Paste before a page's first paint waits for it and keeps its place; a Paste under a dialog runs at once (finding 3); - a Ghostty keybind typed for a new tab runs its action (finding 8); - the 1193 scenario checks the tab each Cmd-D runs on (finding 9); - a key-down that reached nothing has no repeat or release anywhere (finding 10); - an address bar expected by surface takes the keys typed for it (finding 11); - seed 39's shrunk case: a key for the open palette waits for it and always ends (delivered, or dropped when the palette closes). This commit is the red: the router is unchanged, so these tests fail here (the palette and 1193 checks pass: they pin behavior that is right today). * fix(input-router): an ended sink takes keys only as a new sink, creations stay known until their reply, input actions wait like keys, the binding rules read the key's own target (R-INPUT S1 review) Fixes the findings of the first independent review of PR #34052 and the fuzz failure of the S1b green step (fleet step 1803a1b30c2e, seed 39): - Sink lifetimes (finding 1, seed 39): a tab's sink that ended takes no key while its tab still shows, and takes keys again only as a new sink: a window shows it again after none did (Reopen Closed reuses the tab id), or a creation names it again. An ended sink that no window shows and no held key names is forgotten. The router says so with the new SinkEnded and SinkNew effects, and the ledger follows them, so its I7 check holds across a reused id. A window's field, overlay or dialog never ends that way. - Creation replies (finding 2): a creation stays known until its reply (new event Created, or CreationFailed), so a failure after the stand-in showed still ends its terminal. Creation carries the tab's kind. - Input actions (finding 3): an action that edits an input (Paste) waits in that input's lane while the input cannot take input, like a key, and runs when a key could go; under a dialog it edits the dialog's input at once. - Lane limit (finding 7, R9 R-L1): LaneLimit::for_memory derives the bound from physical memory (1/4096 of it, at 1 KiB a waiting key: 4096 keys on 16 GiB); the platform reads hw.memsize at start. - Binding rules (finding 8): the tier and Ghostty rules read the key's own target (the tab the focus is on its way to), the same state as its sink; a page gets a chord first only when the key is aimed at that focused page. - A press that reached nothing (finding 10): no target, dropped, or its sink became a new one: its repeats and release are consumed. - Expected targets (finding 11): a chrome target only on a browser tab, as the focus reducer keeps it; a creation known by its surface keeps its tab. - The ledger keeps only what can still change (finding 6), and names its two protocol checks OwnCall and IdOrder. - Tests: the fuzz runs 20,000 cases of 200 events on every core, with a lane limit of 8, new actions (creation replies after the show, Reopen Closed with the same id, an end while the tab shows, chrome targets, surface keys), ids that never repeat by chance, and a finale the checker adds itself (the shrinker can no longer cut it off, which made seed 39's shrunk case look like a waiting key). It compares the action and tab of every run (finding 9). Two more mutants (I7: a key to an ended sink; I8: an unbounded lane), and the mutant test checks that the real router passes each case first. A counting allocator checks 0 allocations per key, release and bound chord. The router splits into lane, lifetime and binding modules. * fix(input-router): the fuzz finale's window list uses a match guard (clippy collapsible_match) Green step 05d9b051358a at 5ddd5d8 passed fmt and all 56 key_router:: tests (the 20,000-case fuzz, the 0-allocation test, the 13 mutants) and failed clippy -D warnings on one collapsible_match in the test target (fuzz.rs:814). No behavior change. * test(input-router): exits that stay ended, creations done before their tab shows, offers per page with a visible end, bare keys under a dialog (R-INPUT S1 second review), red Scenarios for the findings of the second independent review of PR #34052 (receipt #34052 (comment)): - an exited terminal takes no key after a workspace switch, a tab move or a window reopen, until a restart names it again; an exited terminal whose tab closes is a new tab when reopened (finding 2); - a creation done before its tab shows keeps the keys typed for its terminal (finding 1); - a page that does not report unhandled keys (WebKit) gets no offer: its chords run at once (finding 3); - the offers waiting for one page are bounded by the lane limit and the oldest past the bound closes visibly; a page that closes closes its offers; a handled report closes the offer and the page's older ones (finding 4, R9 R-L1/R-L5); - one page's report keeps the offers of another page (finding 5); - a bare bound key under a dialog goes to the dialog (finding 6); - a Paste for an exited terminal is dropped like a key (finding 7); - copy mode is the focused terminal's, not a new terminal's; the release of an offered key reaches its page; a Paste under a dialog names no tab (nits). The API gets what the tests name: GoneReason::Exited, the PageHandled event, the OfferClosed effect with OfferEnd, and KeyRecord.page_reports_unhandled. The ledger checks I4 on key-downs (a key-down typed while a session runs is the session's unless the session ended first in the call) and accepts an offer's close. The reference model and the fuzz state the new rules; the generator adds replies before the show, exits, restarts, workspace switches and handled reports. This commit is the red: the router is unchanged, so these tests fail here. * fix(input-router): an exited terminal stays ended until a restart, a done creation keeps its keys until its tab shows, offers go only to pages that report and end once per page (R-INPUT S1 second review) Fixes the findings of the second independent review of PR #34052: - Exits (finding 2): GoneReason::Exited is an end whose tab stays (a terminal's process ended). The sink stays ended wherever its tab shows (a workspace switch, a tab move, a window reopen) until a creation names it again (a restart) or its tab closes. Only a closed or failed sink becomes new when a window shows it again after none did (Reopen Closed). - Creations (finding 1): a done creation is kept until a window shows its tab, so a reply that comes before the store's delta never sends the keys for the new terminal to the old target. One reply a creation: a failure after "done" changes nothing. - Offers (findings 3-5, R9 R-L1/R-L5): only a page that reports the keys it leaves unhandled (KeyRecord.page_reports_unhandled, the app's KeyFacts.pageReportsUnhandledKeys) gets a chord first, so a WebKit page's chords run at once as today. Each offer ends exactly once: its action runs, or OfferClosed says why not (Handled, PageGone, Overflow). A page's answer (PageUnhandled, or the new PageHandled) closes only that page's older offers. The offers waiting for one page are bounded by the lane limit (from physical memory), and the oldest past it closes visibly. - A bare key under a blocking dialog is the dialog's, bound or not; a Command or Control chord still runs (finding 6). - An input action for an ended input is dropped with its reason, like a key (finding 7); under a dialog it names no tab. - Copy mode is the focused terminal's, never a new terminal's; the release of a key-down a page got first goes to that page after a repeat that ran; readiness after SinkNew is documented (nits). * test(input-router): a newer key's report closes the older offers as handled, visibly (R-INPUT S1 second review) Green step 2dfb26f0dc2c at f7edc24: fmt and clippy -D warnings on all targets passed, and 67 of 68 key_router:: tests passed (the 20,000-case fuzz, the 13 mutants, the 0-allocation test and the four regression tests of the second review). The one failure was this S1b scenario: it asserted the old rule, where a newer key's report made the older offers vanish with no sign. DESIGN.md 4.7 (v1.3) and R9 R-L5 say every offer ends exactly once, visibly: the older offer closes with OfferClosed { Handled }, and its action never runs. The router is right; the test now asserts that rule. * test(input-router): a failed restart keeps an exited terminal ended; a blocking dialog takes bare keys before a session (R-INPUT S1 third review), red The third independent review of PR #34052 (at 679b1d1, receipt #34052 (comment)): - major: "Restart Shell Here" on an exited terminal's tab is a creation of the same terminal. When it fails, the dead tab stays, but the router records the end as CreationFailed, which follows the closed-tab rule: after a workspace switch, a tab move or a window reopen the terminal is new again, and keys go to a shell that does not run. Scenario a_failed_restart_of_an_exited_terminal_keeps_it_ended_wherever_it_shows. The fuzz: a restart's failure keeps its tab (World::restart marks its reply), so later seeds reach this path. - minor: a running key session took bare keys typed under a blocking dialog; today the dialog gets them first. Scenario a_blocking_dialog_takes_bare_keys_before_a_running_session. This commit is the red: the router is unchanged. * fix(input-router): a failed restart leaves an exited terminal exited; a blocking dialog takes bare keys before a key session (R-INPUT S1 third review) Fixes the two findings of the third independent review of PR #34052: - A creation remembers that it restarts an exited sink. When it fails, the dead tab stays, so the sink ends as Exited again, not CreationFailed: it stays ended wherever its tab shows (a workspace switch, a tab move, a window reopen) until another restart works. - A blocking dialog over the key's target takes a key with no Command or Control before any key session and any binding (dialog_first, DESIGN.md 4.4 step 3, v1.4): today's order, and Chrome's (a page's alert blocks its hint script). The session keeps running and takes its keys again after the answer. A key of another window still ends a running session first. The ledger's I4 check accepts a dialog's key while a session runs. The reference model states both rules. DESIGN.md section 8 lists the behavior changes of S1. * test(input-router): the ledger requires the offers of a page to end in key order (R-INPUT S1), red At dce7c8c the ledger accepted an offer's OfferClosed but did not require it: an older offer that vanished with no end (the expectation that 679b1d1 replaced) passed the ledger. DESIGN.md 4.7 says a page answers its keys in key order and every offer ends exactly once, so the offers to one page must end in key order (I3). This test feeds the ledger both streams directly, with no router and no model: the silent one must break I3 at the newer key's run, and the router's order (the older offer closed as handled, then the newer key's run) must pass. This commit is the red: the ledger is unchanged. * fix(input-router): the ledger requires the offers of a page to end in key order (R-INPUT S1) The ledger keeps each open offer's page, and an offer's end (its action's run, or its OfferClosed) while an older offer to the same page is still open breaks I3. A page answers its keys in key order (DESIGN.md 4.7), so an older offer that vanished with no end can no longer pass: the check that the PR body claimed now exists. Every end the router emits is in that order (an answer closes the page's older offers before it runs or closes its own key; a page's end and an overflow close the oldest first). * fix(input-router): the ledger order test passes its one-effect slice with slice::from_ref (clippy cloned_ref_to_slice_refs) Green step fd523cb817e1 at 75ed18a passed fmt and 71 of 71 key_router:: tests, and failed clippy -D warnings on this one line of the new ledger test (sessions.rs:634). No behavior change.
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
Follow-up of #32291 (its review Low: keys held for two closes could arrive out of typing order). The branch was cut from #32291's head and has feat-cmux-next (with #32291's squash, f0e6822) merged in, so the diff is only this change.
Keys typed during a hidden Cmd-W could reach the terminal out of typing order. Steps: Cmd-W on an idle terminal (it hides while the daemon checks it), type
ab, a quick second Cmd-W on the neighbour that runs a program, typecd. If the neighbour's question shows before the first close's check ends, Cancel typedcdabinto the neighbour.Root cause: a close's question took its keys out of the window's hold (
parkClose), and a close that ended while the question asked appended its keys after them. Cancel then typed the joined list at once, out of the hold.Now the window's hold buffer is the one ordered key queue:
CreationInputBuffer.answering).settleClose(_:keysTo:stays:machine:)): Cancel, the terminal it asked about; Close, the focused one (or, while another close still reads, that close, as before).ParkedClosenow carries only the ticket.CreationInputBuffer.heldKeys(_:for:)had no other caller and is removed.cancel(tab:)keeps the keys of a close that still reads or asks (they have no terminal yet). App lines: -8.The other Low of #32291 (no forced end of a hold whose 2-second limit ran out during a question) is not changed. An answer to a question that held keys now arms the limit again, but an answer to a question with no keys does not.
Testing
Fleet,
cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext ...:keysHeldForTwoClosesArriveInTypingOrderx5 at the test-only commit 1bc38a1, step ed36c389ecef: 5 of 5 fail. The version of the test that reads the screen (8c111e1's test on the red commit, never-mergecp-scratch/sk2-order-red2-never-merge69b2aea), step 8203ca7e3094: 5 of 5 fail, each with the screeno2…o1…for typedo1…o2….Observed / Not run
scripts/cmux-next/prepush-gate.pybefore each push.Changelog
Proof
No capture: no visible UI change (key delivery only); the fleet tests above are the evidence.