Repository navigation
R-INPUT S1: the one input router in cmux-focus-core (core, lanes, sessions, dialogs, page-first chords), Rust only - #34052
Conversation
…h 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.
…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>
…ting 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).
…o_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.
|
Found 43 test failures on Blacksmith runners: Failures
...and 33 more test failures. View all on Blacksmith
|
|
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)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on ee78bc6 to compare; do not merge until green; see lane-rules Shipped in:
|
…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.
…ets, 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).
|
Review (independent subagent, correctness first): FIX at d039085 Paths are relative to the repo root at d039085. The design is /Users/lawrence/fun/wt/handoff/rewrites/R-INPUT/DESIGN.md. I removed the review worktree. Findings1. major: an ended sink stays "gone" forever, so a sink that comes back cannot take keys. The set also grows without bound.
2. major:
3. major: a Paste for a sink that cannot take input yet runs at once.
4. major: the S1a acceptance in the design is not met, or there is no evidence for it.
5. minor: the reference model copies the router's rules. It is independent only for lane mechanics.
6. minor: the ledger cannot see some ended sinks, and its state grows without bound.
7. minor (R9 R-L1):
8. minor: the binding rule and the sink use two different focus states.
9. minor: no test checks which tab an action runs on.
10. minor: the release of a dropped key-down still reaches the sink.
11. minor:
Checked and sound
Nits
Not verified
|
…it, 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).
…ions 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.
|
Review (independent subagent, correctness first): FIX at 5ddd5d8 Re-review of the full PR at the head: S1b (fb2ea1e red, e3c5be0 green) and the fixes for the first review (94527cb red, 5ddd5d8 green). Paths are relative to the repo root at 5ddd5d8. The design is /Users/lawrence/fun/wt/handoff/rewrites/R-INPUT/DESIGN.md v1.2. I compiled and ran nothing. Findings1. major: a creation's
2. major: an ended terminal becomes a "new" sink after any topology that does not show it, so after a workspace switch, a tab move or a window close and reopen, keys go to a terminal that does not run.
3. major: page-first offers do not check that the page reports unhandled keys, so on a WebKit page every content-tier chord and Find is lost.
4. minor (R9 R-L1, R-L5):
5. minor: one page's report removes the older offers of every page.
6. minor: a bare key with a binding runs its action under a blocking dialog instead of going to the dialog.
7. minor: an input action for an ended sink runs, but a key for the same sink is dropped.
On the reference model (finding 5 of the first review, which the author asked me to judge): a restatement of the rules is enough for lane mechanics: order, losses, doubles, the fixed target and the 13 mutants. It is not enough for lifetime, creation and offer rules. Findings 1, 2, 5 and 7 pass both the router and the model by construction. Add scenarios and generator actions for each of them:
Fixes from the first review, checked
S1b, checked and sound
Evidence
Nits
|
…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.
…r 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.
…n 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.
… 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).
|
Review (independent subagent, correctness first): FIX at 75ed18a Re-review of the delta since my receipt at 679b1d1:
Paths are relative to the repo root at 75ed18a. I compiled and ran nothing. Findings1. blocker: clippy
The question asked: is "the older offer closes before the newer key's Run" the design, or a convenience?It is the design.
Third-review fixes, checked
Evidence (logs read)
Nits
|
…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.
|
Non-green checks at 121ee31, classified before any merge decision. None of them comes from a defect in this change.
The fleet evidence of this change is in the PR body: fmt, clippy |
|
Review (independent subagent, correctness first): SHIP at 121ee31 Delta review only, as asked: 75ed18a..121ee31. That is one commit, and its parent is 75ed18a (checked). FindingsNone. Checked
Carried over from my receipt at 75ed18a
Not verified
|
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. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Where a key goes is decided today in about 40 places in the app, mostly after the key: at a hold's flush, at a replay through
NSApplication.sendEvent, at a page's answer or at a 2 s timer. A later decision reads a later state, so a key can reach a target the person never saw focused (P0 1193, REOPEN-F4/F5, close-then-type, INVERSE-KEY, #32633, the #32537 review rounds). R-INPUT replaces those decisions with one reducer in Rust (DESIGN.md).This is stage S1 (S1a and S1b in one PR): the router in
cmux-focus-core::input, Rust only, with no caller yet.InputRouterowns every window's focus (today's focus reducer state) and decides each key's route in the key's own call: the tab the focus is on its way to (a creation's stand-in), else the focused target. AppKit's dispatch only chooses how the key reaches that sink (its responder, else the sink's own path), never which sink. The binding rules (tier rule, Ghostty rule) read the same target.GoneReason::Exited: a terminal whose process ended, its tab stays) stays ended wherever its tab shows until a restart names it again or its tab closes. A creation stays known until its reply, and a done one until a window shows its tab, so a failure after the stand-in showed still ends its terminal and a reply before the show keeps the new terminal's keys. TheSinkEndedandSinkNeweffects say so.OfferClosedreason; a page's answer closes only its own older offers; the offers per page are bounded like a lane, visibly), and Escape in a field the person moved into from content gives the keyboard back to that content (cmux-next: Escape in the right-sidebar text field does not give the keyboard back to the Chromium page #33049).ledger::Ledgerchecks the invariants over the effect stream (I1-I8: no lost key, no double key, order kept, a session's key reaches no sink, one session, the sink fixed at key time, no delivery to an ended sink, bounded lanes). It keeps only what can still change.Tabgets the host terminal id (terminal). The C topology does not carry it yet: the FFI decode setsNone(the app sends it from stage S3). The JSON shape is unchanged when it is absent.Tests (
cmux-tui/crates/cmux-focus-core/tests/input_router, filterkey_router::):CMUX_INPUT_FUZZ_SEEDreplays it).Red/green, the same command:
cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --os linux --arg test|all --arg key_router:: --ref <sha>.collapsible_matchin the test target (fixed in 55ac76c).-D warningson all targets, 56 of 56key_router::tests (the 20,000 x 200 fuzz in 3.3 s, the 0-allocation test, the 13 mutants).-D warningson all targets, 71 of 71key_router::tests (the 20,000 x 200 fuzz, the 13 mutants, the 0-allocation test, every regression test above).ported::). The head changes no FFI code.Regression tests for the second review (each red on the 5ddd5d8 router, green on the head; the fuzz is not the proof):
GoneReason::Exited,InputEvent::PageHandled,InputEffect::OfferClosed,KeyRecord.page_reports_unhandled), with no behavior change. The tests cannot compile on 5ddd5d8 itself, because they use that API.scripts/ci/cmux-tui-rust-check.sh --os linux --arg test|all --arg key_router::. Red step a98b855ab62cb4c8d20ea307 at 0515bf7; green step 2dfb26f0dc2cca99f6d3c4dc at f7edc24.Createdreply before the tab showskey_router::lifetimes::a_creation_done_before_its_tab_shows_keeps_the_keys_for_its_terminalDeliver { Content("b1") }, expectedDeliver { Host("T9"), Direct }key_router::lifetimes::an_exited_terminal_takes_no_key_after_a_switch_or_a_reopen_until_a_restartDeliver { Host("T2") }, expectedDropped { SinkGone(Exited) }key_router::sessions::a_page_that_does_not_report_unhandled_keys_gets_no_offerOffer, expectedRun { browserReload }key_router::sessions::offers_past_the_bound_close_visiblyOfferClosed, expected[(1, Overflow)]Red step a98b855ab62c: 68 run, 14 failed on their assertions (the 4 above, the other 9 new scenarios, and the fuzz at seed 5), 54 passed. Green step 2dfb26f0dc2c at f7edc24: fmt and clippy passed; 67 of 68 passed, with the 4 above passing. Green step 543452f980475a65a96bb360 at 679b1d1: passed, 68 of 68.
One test expectation changed to match the new code (679b1d1), so here is the design rule that makes it right. The S1b scenario
key_router::sessions::a_report_of_a_newer_key_means_the_older_offers_were_handledasserted that a newer key's report makes the older offer vanish with no effect. It now assertsOfferClosed { id: older, reason: Handled }before the newer key'sRun. Both say "handled": the older offer's action never runs in either version. The only change is that the end is now visible. The rule is DESIGN.md 4.7 (v1.3), "Page-first chords":R9 R-L5 asks for the same: "a limit that drops user data spills to disk or shows the loss". The ledger enforces it from 75ed18a on (before, it only accepted the close and did not require it): it keeps each open offer's page, and an offer's end while an older offer to the same page is still open breaks I3.
key_router::sessions::the_ledger_requires_the_offers_of_a_page_to_end_in_key_orderfeeds the ledger both streams with no router and no model: the old silent stream must break I3, and the router's stream (the older offer closed as handled, then the newer key's run) must pass. Red 1525c55 (step 3b52c986936b): this one test failed on its assertion (the old ledger reported no violation for the silent stream), 70 of 71 passed. Green 75ed18a (step fd523cb817e1): fmt and 71 of 71 tests passed; clippy failed on one line of this new test (cloned_ref_to_slice_refs), fixed in 121ee31 withstd::slice::from_ref.Third review (at 679b1d1, receipt): FIX, one major, one minor; red 6d4725c and green dce7c8c fix both.
key_router::lifetimes::a_failed_restart_of_an_exited_terminal_keeps_it_ended_wherever_it_showsDropped { SinkGone(CreationFailed) }, expectedSinkGone(Exited)(step cc5b682f6dcf)key_router::sessions::a_blocking_dialog_takes_bare_keys_before_a_running_session[1]Red step cc5b682f6dcf: 70 run, the 2 above failed on their assertions, 68 passed. Green step ce4553b5b2b1 at dce7c8c: fmt, clippy
-D warningson all targets, and 70 of 70key_router::tests passed.The fuzz reaches the failed restart too:
World::restartmarks its reply, and a failed restart keeps its tab.The fuzz event alphabet has the three cases, so later seeds can reach them (
fuzz.rs):World::reply: half the replies are aCreatedfor a creation whose stand-in has not shown yet.World::end_shownsendsGone { Exited }while the tab shows;World::switch_awayshows another workspace, action 105 shows the window's own workspace again, action 98 closes and opens window w2, andWorld::restartnames a shown terminal again.World::keysetspage_reports_unhandled: falseon 30 % of keys.Review: the first independent review (at d039085) found 4 major and 7 minor defects (receipt); 94527cb and 5ddd5d8 fix them. The second (at 5ddd5d8) found 3 major and 4 minor defects (receipt); 0515bf7, f7edc24 and 679b1d1 fix them. The third (at 679b1d1) found 1 major and 1 minor defect; 6d4725c and dce7c8c fix them, and DESIGN.md v1.4 takes its three nits (the reload of a crashed page, and the S3/S4 caller checklist). The fourth review stopped on a connection error before its verdict; its last note found that the ledger did not require an offer's close, which 1525c55 and 75ed18a fix. The fourth (at 75ed18a, receipt) found only one clippy line in the new ledger test, fixed in 121ee31, and confirmed that the changed offer expectation is the design. The review of that delta: SHIP at 121ee31 (receipt).
Observed / Not run
tooling-testsand the concurrency part ofcmux-next checksare base findings;Check the app FFI pinis this PR's own and expected, becausecmux-focus-coreis an app FFI input and the release is published only after the feat-cmux-next push (the bot then opens the app-ffi-repin PR; agent cursor visibility: a typed C ABI call for the per-frame path (B9b step A) #32564 and fix(focus-core): a record for the workspace the window already shows focuses its saved pane (QA v2.2.2 bug 1, step A) #32055 landed the same way).input.router.shadow. No Swift change here. The app FFI decode change sets one new field toNone, so the app's behavior is unchanged; the FFI release workflow will publish a new pin.CMUX_INPUT_FUZZ_FIRSTandCMUX_INPUT_FUZZ_CASES).Changelog
none
No capture: Rust-only library code with no caller; nothing in the app changes.