Repository navigation
Focus: a focus choice the person makes from a menu counts as their move (the reply bound drops a held line after it) - #35874
Conversation
…eirs (red) Recorded on dogfood build 7dd31211 (e60197e, STATUS 12884): a main-menu item chosen with a real mouse ran its action outside any input event's dispatch, so its focus events carried the CLI's or the app's source, and FocusState.personMoveSeq (the count #35275's reply bound compares) did not move. A line held for a stalled Cmd-N's terminal was then typed into that terminal at the bound although the person had moved the focus from the menu. MenuPickPersonMoveTests: Focus Next Pane's menu item, its action sent as AppKit sends a chosen item's (outside a dispatch), moves the focus and must count as the person's (fails on feat 4a55412: the count does not move); the same move from the CLI must not count (passes).
Recorded on dogfood build 7dd31211 (STATUS 12884): a menu item chosen with the mouse runs its action outside any input event's dispatch, so its focus events carried the CLI's or the app's source and FocusState.personMoveSeq did not move; #35275's reply bound then typed a held line into the press-time terminal although the person had moved the focus from the menu. - ActionMenuTarget.performAction marks a run the person chose from an open menu (the main menu or a context menu) outside a key's dispatch: ActionInvocation.menuPick, carried by ActionRunScope (and kept by the runs nested in it). A key equivalent is not marked: its own key dispatch names the person, and the round-10 rule of #35275 stays (a key's responder change is no focus choice). - FocusMachine.send counts any focus choice (focusPane, selectTab, focusTarget, a responder report) of such a run as the person's move. FocusCoordinator passes the current run's mark. - No source the focus core reads changes (its drag and sidebar-list rules read sources), so the reducer behaves as before; no FFI change. An accessibility press on a menu item also counts, as ActionOrigin names menu runs the user's; it fails safe (held keys dropped and logged). Test: MenuPickPersonMoveTests (red b7b37b6).
|
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:
|
|
Review (independent subagent, correctness first): FIX at 0e5a693 I reviewed the two commits against feat 4a55412 by reading the code only. I ran no build or test here. The fix is correct for the path it names. A main-menu item, or a context-menu item the registry built, that is picked outside a key dispatch gets the mark ( 1. Fix: the guard does not cover the recorded failure (a responder report)The PR body records View > Focus Right Sidebar, picked with the mouse on 7dd31211. Its only focus event was
Please add a case that sends Focus Right Sidebar's menu item in the same way ( 2. Note: the mark follows the task tree into key dispatches
3. Note: menus whose items run closures are not coveredOnly 4. Note: the contract next to the counter is out of dateThe doc on Checked, no finding
|
…counts as the person's; a key equivalent's run is no menu pick Review of #35874 at 0e5a693 (finding 1): the round-1 test counts through Focus Next Pane's focus move, so it did not cover the recorded case (dogfood build 7dd31211, STATUS 12884), whose one focus event is a responder report with the app's source: removing `.responder` from the menu pick rule left every suite green. MenuPickPersonMoveTests gets: - aResponderMoveChosenFromAMenuCountsAsThePersons: View > Focus Right Sidebar's menu item, its action sent as AppKit sends a chosen item's; the sidebar's search field reports as the first responder and the count must move (fails on feat 4a55412: nothing counts it). - aKeyEquivalentRunIsNoMenuPick: the same menu path run from a key-down's dispatch (`isDispatchingKeyDown`) moves the focus and must not count (fails if every run is marked). The harness keeps the right sidebar's state in a test defaults suite.
…choice (red) Review of #35874 at 0e5a693 (finding 2): the menu pick mark is a task local, so it also covers a held key that a creation hold runs again inside the menu run's task (File > New Workspace by mouse: the hold's bound task inherits the mark). FocusMachine then counted that key's responder report as the person's choice, which round 10 of #35275 forbids. FocusPersonMovesTests.aMenuPicksChoicesCountButAKeysResponderReportDoesNot: under `menuPick` a responder report with the app's source and a CLI focus move count; a responder report with the keyboard's source does not (fails on 0e5a693).
…de another run stays marked Review of #35874 at 0e5a693: - Finding 2: the menu pick mark is a task local, so it also covered a held key that a creation hold ran again inside the menu run's task, and FocusMachine counted that key's responder report as the person's choice. FocusEvent.choosesFocusInMenuPick replaces choosesFocus: inside a menu pick every focus or selection move counts, and a responder report counts only when no key's dispatch made it (its source is not the keyboard's). A key's own focus actions still count by their source, as before. - ActionRunScope.nested(in:) keeps a menu pick made inside another run (`outer || inner`). - Finding 4: the docs of FocusState.personMoveSeq and FocusMachine.send name the menu pick rule. Tests: MenuPickPersonMoveTests (the recorded Focus Right Sidebar case and the key equivalent guard, 3a3a39d) and FocusPersonMovesTests.aMenuPicksChoicesCountButAKeysResponderReportDoesNot (red 0e213dd). Round 1: red 2e11c857, green 47ce629a.
|
Review (independent subagent, correctness first): SHIP at 8b2b51e Round 2. I reviewed 3a3a39d, 0e213dd and 8b2b51e on top of round 1 by reading the code only. I ran no build or test here. Nothing blocks the landing. Findings 1 and 4 are resolved. Finding 2 is narrower now but not removed (note A). It can only add a count, which drops a held line and logs it, and I find no delivery decision that it changes. Finding 3 stays a documented limit, which is acceptable, but the stated reason is only partly right (note B). Round 1 findings
A. Held keys still run again under the menu mark, and the source check does not catch them (not blocking)
B. The reason for not using one
|
…ide the opening run's task locals (review of #35874) (#35935) * test(focus): a creation hold's reply bound and limit end the hold outside the task-local scope of the run that opened it (red, review of #35874 finding 2) * fix(focus): a creation hold's reply bound and limit end the hold in a turn of their own, outside the opening run's task locals (review of #35874 finding 2) * fix(focus): the hold timers sleep off the main actor and end the hold in a main-actor method (the region checker refused a main-actor detached closure with a weak capture)
REWRITE R-INPUT. A focus choice that the person makes from a menu now counts as their move (
FocusState.personMoveSeq), so #35275's reply bound no longer types a held line into the press-time terminal after the person moved the focus from a menu.Recorded first, before any code change (lead order after #35275 landed), on FEAT DOGFOOD BUILD 7dd31211 (e60197e), cmux18, whole host: real desktop mouse clicks (CuaDriver) on the menu bar's View and then on Focus Right Sidebar. The action ran (
focusRightSidebar, committed, action-log entrypointunattributed), and the window's only new focus event wasresponder(.textField, source: .programmatic). AppKit runs a chosen menu item's action after the menu's tracking, outside any input event's dispatch, soFocusEvent.Source.currentis programmatic and a handler's.intentis the CLI's: neither counts as the person's choice. (STATUS 12884; runmm4-e60197eb-20261104T210106Z-cmux18.)What changes:
ActionMenuTarget.performActionmarks a run the person chose from an open menu (the main menu or a context menu) outside a key's dispatch:ActionInvocation.menuPick, carried byActionRunScopeand kept by the runs nested in it. A key equivalent is not marked: its own key dispatch names the person, and Creation hold: a daemon reply keeps the window's keys at most 10 s (its own bound), and no hold end drops a key (cmux-ad m12) #35275's round-10 rule (a key's responder change is no focus choice) stays.FocusMachine.sendcounts a focus choice of such a run as the person's move (FocusEvent.choosesFocusInMenuPick): anyfocusPane,selectTaborfocusTarget, and a responder report that no key's dispatch made (its source is not the keyboard's).FocusCoordinatorpasses the current run's mark. A run started inside a menu pick keeps the mark (ActionRunScope.nested); a held key that a creation hold runs again inside the menu run's task is judged by its source, so a responder change with the keyboard's source is no focus choice (Creation hold: a daemon reply keeps the window's keys at most 10 s (its own bound), and no hold end drops a key (cmux-ad m12) #35275, round 10). This only narrows that case (review round 2, finding 2): see Known limits.Tests
MenuPickPersonMoveTests(Packages/macOS/CmuxNext), red first (b7b37b6 on feat 4a55412):aFocusMoveChosenFromAMenuCountsAsThePersons: Focus Next Pane's menu item, its action sent as AppKit sends a chosen item's (outside any dispatch), moves the focus and must count as the person's.theSameMoveFromTheCLIDoesNotCount: the same move from the CLI (focus: true) must not count.aResponderMoveChosenFromAMenuCountsAsThePersons(round 2, the recorded case): View > Focus Right Sidebar's menu item; the sidebar's search field reports as the first responder with the app's source, and the count must move.aKeyEquivalentRunIsNoMenuPick(round 2): the same menu path run from a key-down's dispatch moves the focus and must not count.FocusPersonMovesTests.aMenuPicksChoicesCountButAKeysResponderReportDoesNot(round 2): under the mark, a responder report with the app's source and a CLI move count; a responder report with the keyboard's source does not.Compile checks pass: 5db465d9 (head 0e5a693), 039b513a (red b7b37b6). Red: step 2e11c8579ee1265d870d4edc (
MenuPickPersonMoveTests --redat b7b37b6, aws-m4pro-3): exactly the new case fails,aFocusMoveChosenFromAMenuCountsAsThePersonsat MenuPickPersonMoveTests.swift:64 ("(personMoveSeq → 0) != (before → 0)": the focus moved, the count did not);theSameMoveFromTheCLIDoesNotCountpasses. Green: step 47ce629abd6857411d6ab30d at 0e5a693 (aws-m4pro-6): all 7 suites pass (MenuPickPersonMoveTests, FocusPersonMovesTests, CreationHoldReplyBoundTests, CreationHoldReplyBoundRuleTests, PersonTerminalKeyboardClaimTests, MenuKeyRoutingTests, FocusWaitLeavesPageTests).Merged tree: this head merged with feat b5e96a1 (after #35875, which moved
FocusMachine.sendontocmux_input_*) on a never-merge ref, 61c9441: compile check 45d1cc24 passes, and step de999219fd818d2c35a92883 (aws-m4pro-7) passes all 10 suites (the 8 above plus FocusInputSharedRouterTests and MultiWindowInputRoutingTests). Review round 2: SHIP at 8b2b51e (#35874 (comment)).Review round 1 (FIX at 0e5a693, #35874 (comment)): (1) the test did not cover the recorded responder-report case; (2) the mark also covered a held key run again inside the menu run's task; (3) closure-based menus get no mark; (4) two docs were out of date. Round 2: tests 3a3a39d and 0e213dd (red), fix 8b2b51e; (3) is a known limit with the reason (above). Compile checks pass: a16a09d2 (head 8b2b51e), 5eae1837 (red A 0e213dd), 6d24648b (red B 543fe01, a never-merge branch: feat plus the menu tests, no fix). Red A: step e365d6c0e52765fde7905357 (
FocusPersonMovesTestsat 0e213dd, the new key-report test on the round-1 fix, aws-m4pro-7): exactly the new case fails,aMenuPicksChoicesCountButAKeysResponderReportDoesNotat FocusPersonMovesTests.swift:95 ("(state.personMoveSeq → 3) == 2": a key's responder report inside a menu pick counted); the 5 other cases pass. Red B: step 73dd57404556ebbb5473b359 (MenuPickPersonMoveTestsat 543fe01, aws-m4pro-9): both menu cases fail,aFocusMoveChosenFromAMenuCountsAsThePersons(:68) andaResponderMoveChosenFromAMenuCountsAsThePersons(:85, the recorded responder-report case), "(personMoveSeq → 0) != (before → 0)";aKeyEquivalentRunIsNoMenuPickandtheSameMoveFromTheCLIDoesNotCountpass. Green: step 9e35de97eef1b32c535ab36f at 8b2b51e (aws-m4pro-9): all 8 suites pass (MenuPickPersonMoveTests, FocusPersonMovesTests, CreationHoldReplyBoundTests, CreationHoldReplyBoundRuleTests, PersonTerminalKeyboardClaimTests, MenuKeyRoutingTests, FocusWaitLeavesPageTests, RightSidebarTests).Known limits
debug.menu_itemalso counts:ActionOriginnames menu runs the user's. It fails safe: held keys are dropped and logged, never typed into a wrong place.HomeMenuTarget, the browser context menusClosureMenuTarget, the titlebar history menu, the remote toolbar menu), are not marked: a focus move chosen there still does not count. Not taken here: an override ofCmuxApplication.sendAction(_:to:from:)that bindsActionRunScopefor every menu item would make every registry menu run a nested run, so the top-level-run rule ofActionRouting+SidebarUserIntent(the person's next intent ends their new terminal's keyboard claim) would stop firing for a menu pick. A plain flag onCmuxApplication, set whilesendActionruns with anNSMenuItemsender and no key-down is dispatching (ashandlingSendEventis), has neither problem; it is the likely shape of the fix, and it belongs to R-INPUT S8 (every entrypoint carries its origin into the router). (Round 1 also named the macOS 14 task-local crash class ofTaskLocal+InTask.swiftas a reason; review round 2 showed that it does not apply: below macOS 15.4withValueInTaskstarts no task, and every registry run already uses it.)Task {}(CreationInputBuffer.swift,CreationInputBuffer+ReplyBound.swift;scheduleFlushusesTask.detachedon purpose). The keyboard-source check does not catch every such key: a held shortcut runs again outside any dispatch (.programmatic), and other held keys read a staleNSApp.currentEvent. Example: File > New Workspace with the mouse, then the person types a line and Cmd-F while the daemon does not answer; at the bound, the find field's responder report counts as the person's move. No held key goes to a wrong place (the bound reads the count before the keys run again); the count only moves for a later hold of that window. Follow-up (owned, R-INPUT): both timers detach from the opening run (Task.detached { @MainActor ... }), with a red test first, so held keys run again as the person's own keys and never under another run's scope.unattributed: R-INPUT S8 (entrypoints carry their origin into the router).Observed / Not run
Changelog
Fixed: Keys typed while a new workspace opens slowly no longer run in the old terminal after you pick a menu item that moves the focus.
No capture: no visible UI change (a focus rule).