Skip to content

R-INPUT S2b step B: every window's focus machine keeps its focus in the app's one input router (cmux_input_*) - #35875

Merged
lawrencecchen merged 5 commits into
feat-cmux-nextfrom
fix/r-input-s2b-step-b
Nov 4, 2026
Merged

lawrencecchen merged 5 commits into
feat-cmux-nextfrom
fix/r-input-s2b-step-b

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

REWRITE R-INPUT stage S2, step B of the FFI two-step (step A #35641, re-pin #35853, DESIGN.md section 5): every window's FocusMachine keeps its focus in the app's one input router (cmux_input_*) under a window key of its own, instead of a per-window cmux_focus_* handle. Pure refactor: the same requests and the same reply bytes (step A's corpus test compares them), so every focus rule is unchanged.

What changes:

  • FocusInput (CmuxNextFocus): owns the CmuxInput handle; made once at the composition root (AppServices.focusInput) and given to each window's FocusCoordinator through WindowState. The router's key lanes are bounded by the Mac's physical memory (cmux_input_new(memory)).
  • One window key per machine: each machine opens a window under a key no other machine of that router had, and closes it when it is freed. So a window id the app reuses (a restored closed window) never meets the state of the machine it replaces, whichever is freed first.
  • A machine made with no input (tests, the journal replay InputReplay) gets a router of its own.
  • FocusMachine's Close Tab aim calls move to FocusMachine+CloseAim.swift (the A12 follow-up); FocusMachine.swift goes from 467 to 458 lines.
  • FocusResponderClassifier.describe moves to the package as FocusEvent.Responder.debugName. Arch app_lines -10.

Tests

FocusInputSharedRouterTests (Packages/macOS/CmuxNext, new):

  • aMachineInTheSharedRouterReducesAsAMachineAlone: a machine in the shared router and a machine alone get the same reply (changed, effects, omnibar flags) and the same state after every event of a sequence (topology, person moves, a selection, the address bar, a click report, an intent, a CLI move, a topology change, a palette move), while another window of the same router acts between its events; the same checkpoint, history and remembered panes at the end.
  • aNewMachineNeverMeetsAFreedMachinesFocus: a machine freed before a new one is made leaves no history or remembered panes to it; the window still open keeps its own.
  • everyRequestGoesThroughTheInputHandle: the close-focus policy, the Close Tab aim (pane emptied, pane moved, aimed at, presented), the history, the remembered panes and the checkpoint all go through cmux_input_*; a checkpoint that does not decode is refused and leaves the state as it was (R-INPUT S2b (FFI step A): the app's one input handle cmux_input_* over the input router, with no copy per focus event #35641 review notes: the C ABI test did not call every export, and did not compare the state before and after a bad import).

A pure refactor has no red step: the old path is gone in this PR. The check that the new path answers as the old one is step A's Rust corpus test the_input_handle_replies_to_every_corpus_step_as_the_window_handle (cmux-tui/crates/cmux-layout-reducer-ffi/tests/input_handle.rs, landed in #35641): it replays every corpus sequence (100+ sequences, 1,000+ focus events) through the input handle and through the old window handle and compares each reply. The Swift tests above check the app's side only: a machine in the shared router answers as a machine alone while another window acts, windows stay apart, and every Swift call goes through cmux_input_*.

Compile check 1e9b30f5 of 83c1758 (this branch merged with feat 00655a4) passes; e79c99c changes only the new test file. Green: step e498ede205b71ebe3d903591 at e79c99c (aws-m4pro-4): all 21 suites pass (FocusInputSharedRouterTests, FocusPersonMovesTests, FocusWaitLeavesPageTests, CloseTabKeepsPaneTests, MultiWindowInputRoutingTests, FocusFollowsMouseTests, FocusedPaneShowsFirstTests, KeyRouterRepeatAndChordTests, MenuKeyRoutingTests, PaneCloseApplyTests, PaneCloseControllerTests, WindowFocusRestoreTests, WebViewFocusedContextTests, NewTabHoldFocusTests, CreationHoldReplyBoundTests, CreationHoldReplyBoundRuleTests, PersonTerminalKeyboardClaimTests, HomePendingTerminalFocusTests, SwitchToColdTabTypeAheadTests, HeadlessRenameTakesNoKeyboardTests, HeldKeyRunPanelTests). An earlier run at 83c1758 (step b998c98b) failed one case of my new test everyRequestGoesThroughTheInputHandle: it called the Close Tab aim in an order PaneCloseAim does not document (the frame stamped before the focus moved); e79c99c follows the documented order and compares every answer with a machine alone.

ASan (CMUX_SWIFT_SANITIZE=address, step 085409561b6c6bf6a53fb226): 5 of 8 suites pass (FocusInputSharedRouterTests, FocusPersonMovesTests, FocusWaitLeavesPageTests, WindowFocusRestoreTests, CreationHoldReplyBoundRuleTests: the machine reply reads under the sanitizer). The 3 window suites (CloseTabKeepsPaneTests, PaneCloseControllerTests, MultiWindowInputRoutingTests) abort in the ASan runtime itself before any assertion: "AddressSanitizer failed to deallocate 0x40000 bytes at address 0x...b28 (error code: 22) / Failed to munmap" (a non-page-aligned unmap inside the sanitizer's allocator). Classified as a base finding, not this change: the same 3 suites under ASan on the base feat 00655a4 (without step B, step dc3b33cf482e2f0a0de4c745, aws-m4pro-2) abort with the identical runtime error ("failed to deallocate 0x40000 bytes at address 0x...b28 (error code: 22) / Failed to munmap") in all 3 suites. So under ASan the fleet Macs (macOS 26/27, Xcode 26.6) cannot run these window suites at all, with or without this PR; the 5 suites that do run under ASan include the machine-level ones that read every reply of the input handle.

S2 test phase: the input router fuzz at 1,000,000 cases on the merged step A commit dba5f44 passed (step 85d683ea37e2, Linux, 180.3 s, 0 findings).

Review

Review (independent subagent a95e30fa): SHIP at e79c99c (#35875 (comment)). Its non-blocking findings go to one follow-up PR: test 3 compares the new path with itself and the absolute checks of history and remembered() came out (F1); one check of test 1 cannot fail (F2); test 2 does not prove that deinit closes the window (F3); FocusMachine.deinit must never run off the main thread now that it writes to the shared router (F5); the default FocusInput() of WindowState.init can give a future production window a router of its own (F6); a stale cmux_focus_* comment in Package.swift (F7). F4 (this body named the Swift test as the parity proof) is fixed above.

Observed / Not run

  • Observed: the fleet compile check, the 21 focus and hold suites, and 8 of them under CMUX_SWIFT_SANITIZE=address (the lead's sanitizer plan for the app's reads of a reply: ASan's interceptors cover the Rust library's heap too); the prepush gate (no new violations, app_lines -10).
  • Not run: KeyRouterBench A/B (focus bench within 5 %, DESIGN.md S2 acceptance): it runs on the fleet after this lands, with the stage's test phase.

Changelog

none

No capture: no visible change (a refactor).

manaflow-cmux-ffi-release Bot and others added 4 commits November 4, 2026 19:59
…he app's one input router (cmux_input_*)

Step B of the FFI two-step (step A #35641, re-pin #35853): FocusMachine calls the input handle
(`cmux_input_focus_*`, one window key per machine) instead of a per-window `cmux_focus_*` handle.
The router is made once at the composition root (AppServices.focusInput, FocusInput in
CmuxNextFocus) and given to each window's FocusCoordinator through WindowState. Pure refactor:
the same requests and the same reply bytes (step A's corpus test), so every focus rule is
unchanged.

- FocusInput owns the CmuxInput handle (memory: the Mac's physical memory, which bounds the key
  lanes). Each machine opens a window under a key no other machine of that router had, and closes
  it when freed, so a window id the app reuses (a restored closed window) never meets the state
  of the machine it replaces, whichever is freed first.
- A machine made with no input (tests, the journal replay) gets a router of its own.
- FocusMachine's Close Tab aim calls move to FocusMachine+CloseAim.swift (A12 follow-up); the
  file shrinks from 467 to 458 lines.
- FocusResponderClassifier.describe moves to the package as FocusEvent.Responder.debugName
  (CmuxNextApp: -10 lines).

Tests: FocusInputSharedRouterTests: a machine in the shared router reduces exactly as a machine
alone (state and effects after every event) while another window acts between its events; a new
machine never meets a freed machine's focus; every request goes through the input handle (the
close-focus policy, the Close Tab aim, the history, the remembered panes, the checkpoint; a
checkpoint that does not decode is refused and leaves the state as it was: review notes of
#35641).
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review e79c99c: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr35875 -f ref=fix/r-input-s2b-step-b -f platform=macos -f backend_mode=local

Shipped in:

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41927927728 at b5e96a1 failed: cmux-next checks (god files, concurrency, crash safety, l10n).
Those jobs last passed at 3484771. This pull request is one of 5 merged since: #35886, #35875, #35873, #35889, #35898.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

…umented order (the focus moves, then the frame shows) and compares each answer with a machine alone
@cursor

cursor Bot commented Nov 4, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot 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.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent subagent, correctness first): SHIP at e79c99c

Scope: git diff 00655a40430..e79c99cc59b (11 Swift files). I also read the Rust side these files now call (cmux-layout-reducer-ffi/src/input.rs, src/focus/reply.rs, cmux-focus-core/src/input/router.rs, window.rs) at the pinned FFI b24aac2c507. The FFI crates do not differ between that pin and the base. This was a read-only review: I built nothing and ran nothing.

I found no blocking defect in the production code. All findings below are non-blocking. The test findings (F1 to F3) are worth a follow-up.

Checked and correct

  1. Reply lifetime. Each reply or string list is read before the next cmux_input_* call for the same window:

    • send (FocusMachine.swift:53, read at :58)
    • set (:101, read at :107)
    • recentPanes (:124/:127, read at :129-130)
    • remembered (:135)
    • restore (:156 import, then :163 current and read)

    read and mirror make no FFI call. Each window has its own boxed reply (input.rs:26-29), and every reply pointer points into the vectors of that box (reply.rs write). So when another window opens or closes, this window's reply does not move (Rust test a_windows_reply_does_not_move_while_other_windows_open_and_close). No reply is held across a machine's deinit.

  2. Ownership. Each machine holds its FocusInput strongly (FocusMachine.swift:14). Its deinit closes the window before input is released, so cmux_input_free always comes after the last cmux_input_window_close. There is no retain cycle.

  3. Window identity. Keys come from a counter in each FocusInput (m1, m2, ...; FocusInput.swift:23-24). Two machines never share a key, and a reused app window id never meets the state of a live or freed machine.

    • Both production construction sites pass services.focusInput: state(for:) at WindowManager.swift:205 and the restore path at :275. No other production code makes a WindowState.
    • Tests and InputReplay (InputReplay.swift:64) get a router of their own. That is correct for S2, because the router keeps no focus state that two windows share.
  4. Threads.

    • AppServices, WindowState, FocusCoordinator and the app tests are main-actor (uiSwiftSettings default isolation).
    • debug.replay is a .mainActor handler (AppControl+Input.swift:19).
    • FocusInput is not Sendable, so the compiler keeps it inside those main-actor owners.
    • The one detached task near the coordinator captures it weakly (FocusCoordinator.swift:180).

    I found no path where two threads call one handle at the same time. See F5 for one latent case.

  5. Parity. Each former cmux_focus_* call has its cmux_input_focus_* twin, with the same arguments in the same order: send, set_topology, current, set_close_focus, recent_panes (both forms), remembered, export/import json, pane_moved (both forms), frame_presented, pane_emptied, aims_at_emptied_pane. cmux_focus_changes_view and cmux_focus_string_free stay; neither takes a handle. On the Rust side:

    • For an open window, write_reply always finds the FocusReduced that the router pushes (router.rs:414).
    • WindowFocus::send clears its effects first (window.rs:57), and the router drains them on each call.
    • focus_import resets the reply, as Focus::import did.
    • drain, drop_left_lanes, track_gone and session_after_focus do not change focus while no key or session goes through the router.

    Byte parity with the old handle is proven by step A's corpus test (see F4).

  6. Moved code. FocusEvent.Responder.debugName returns the same eight strings as the removed FocusResponderClassifier.describe. FocusMachine+CloseAim.swift makes the same four calls with the same arguments.

  7. ASan. The base run has finished. Step dc3b33cf on 00655a4, which does not have step B, aborts the same 3 suites with the same error: failed to deallocate 0x40000 bytes at 0x...b28 (error code: 22) / Failed to munmap. The abort comes before any assertion, and CloseTabKeepsPaneTests and MultiWindowInputRoutingTests abort in the same first test as on head. So this is a base finding.

    I see nothing in this diff that could cause it:

    • The diff starts no thread, makes no mmap and makes no large allocation. The router starts with empty maps, and LaneLimit::for_memory only computes a count.
    • The Rust library is the same pinned binary at base and head.
    • The unmapped address is always a page base plus 0xb28. An unmap of an address that is not page-aligned is the sanitizer's own bookkeeping, not a report about user memory.

    FocusInputSharedRouterTests, which opens, uses and frees routers and windows, passes under ASan.

Findings (non-blocking)

F1. Test 3 compares the new path with itself, and the rewrite removed its absolute checks.

  • FocusInputSharedRouterTests.swift:85: alone = FocusMachine() is FocusMachine(input: FocusInput()). That is the same setup as inRouter: two one-window routers over cmux_input_*. So the comparison at :110 (answers["router"] == answers["alone"]) cannot fail on a parity break.
  • The 83c1758 version of this test checked recentPanes(in: "w").prefix(2) == ["a", "b"] and remembered()["w"] == "a". e79c99c removed both checks.
  • Scenario: if cmux_input_focus_remembered returned null for the window, remembered() would give [:] on both machines, and the test would pass. After this change, no Swift test checks remembered(), which WindowRecordSaver.swift:209 saves as the window record's focused panes.
  • The "state kept" check (:105) can also pass vacuously: checkpoint() gives "{}" on a null export, and "{}" matches "{}".
  • Fix: keep the comparison and add the absolute checks back: the history, the remembered panes, and a checkpoint that is not empty.

F2. Vacuous assertion at FocusInputSharedRouterTests.swift:56.

  • Swift writes otherState.topology from the topology argument (FocusMachine.swift:109), never from a router reply. So this line cannot detect this window's events leaking into the other window, although its message says it does.
  • Lines 43-52 do test isolation in one direction: inRouter must equal alone while other acts between its events.
  • Fix: send the other window's events to a third machine alone too, then compare other.checkpoint() and other.recentPanes(in: "other") with that machine's.

F3. Test 2 does not test the teardown (FocusInputSharedRouterTests.swift:61-78).

  • Keys never repeat, so fresh passes even if FocusMachine.deinit never called closeWindow. In that case each freed machine would leave its window in the router.
  • Fix: with @testable import CmuxNextFocus and import CCmuxLayoutReducerFFI, copy closing.window inside the do block. After the block, assert that cmux_input_focus_current(shared.handle, ...) with that key returns nil. The header says a window that is not open has no reply.

F4. A claim in the PR body is not accurate.

  • The body says: "The equivalence test above is the check that the new path answers as the old one, event by event." No Swift test runs the old cmux_focus_* handle.
  • The proof from old to new is step A's Rust corpus test: the_input_handle_replies_to_every_corpus_step_as_the_window_handle (cmux-tui/crates/cmux-layout-reducer-ffi/tests/input_handle.rs:244). Please cite that test in the body.
  • If you want a check at the Swift level, the pinned FFI still exports cmux_focus_new, so it can be the "alone" side.

F5. A latent thread hazard; I found no path to it today.

  • FocusMachine.swift:32-34: deinit now writes to the shared router (a HashMap remove and lane drops). Before this change it freed a private handle, which was safe on any thread.
  • Scenario: if the last strong reference to a WindowState or FocusCoordinator is ever released off the main thread (for example, by a detached task that holds it strongly), that window close races the main thread's calls into the same router.
  • Optional fix: a debug dispatchPrecondition(condition: .onQueue(.main)) in FocusInput.closeWindow, or a line in its doc that says "release on main only".

F6. A default argument can hide a private router.

  • WindowState.swift:102 and :118 set focusInput to a new FocusInput() by default. In S2 this changes nothing.
  • Scenario: a future production call WindowState(id:) would get a router of its own without any warning, and in S3 that window's keys would not route through the app's router.
  • Fix: make the parameter required in production code, and give the tests a helper.

F7. Stale comment. Package.swift:99 still says "FocusMachine over cmux-focus-core's cmux_focus_* C ABI".

Not run: I did no build and ran no tests (this was a read-only review). The focus bench from the DESIGN S2 acceptance ("focus bench within 5 %") has not run, as the PR body already says.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Swift logic follow-up (tip watcher swift-logic row, STATUS 13027 "SWIFT LOGIC CULPRITS"; lead order).

This PR raised swift_logic_lines by +67 at its merge commit c6be943: scripts/cmux-next/swift-logic-lines.py before and after the merge, with the same table (the tip's, c6be943). The largest declarations of the rise:

  • class FocusInput in Packages/macOS/CmuxNext/Sources/CmuxNextFocus/FocusInput.swift: +27 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)
  • extension FocusMachine in Packages/macOS/CmuxNext/Sources/CmuxNextFocus/FocusMachine+CloseAim.swift: +24 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)
  • extension FocusEvent.Responder in Packages/macOS/CmuxNext/Sources/CmuxNextFocus/FocusEvent.swift: +15 lines counted as Swift logic (no row in plans/cmux-next/swift-logic-classes.tsv)
  • class FocusCoordinator in Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/FocusCoordinator.swift: +1 lines counted as Swift logic (its row counts it as logic)

plans/cmux-next/layer-ownership.md says Rust owns every state, list and rule and Swift is the platform shell. Please move the new logic of this PR in a follow-up: to Rust (the crate that owns the area; the owner lists in STATUS 12147-12165 name the target per area), or at least out of the app into its package. If a declaration above is platform code (AppKit, Core Animation, OS calls, strings, tokens, DEBUG tooling), reply here and the table gets a PLATFORM row with your reason instead.

The swift-logic row is a report, not a gate: nothing blocks on this comment.

lawrencecchen added a commit that referenced this pull request Nov 5, 2026
…indow close; WindowState takes the router with no default (review of #35875) (#35924)

* test(focus): the shared-router tests check absolute answers, the other window's focus and the window close; WindowState takes the router with no default; a machine freed off the main thread stops debug builds (review of #35875: F1-F3, F5-F7)

* test(focus): the shared-router test also checks the shown workspace's history (review of #35924, finding 1)

This branch was previously deployed

1 inactive deployment
ci-routing-agent — e79c99cc Deployed Nov 4, 2026 by lawrencecchen via namespace autofix #14617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant