Skip to content

fix(focus): pin CCmuxAppFFI to #30356's release: a sidebar AppKit picks never takes a restored terminal's keyboard (REOPEN-F4, F5) - #31684

Merged
lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/p0-reopen-f4-ffi-repin
Nov 2, 2026
Merged

lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/p0-reopen-f4-ffi-repin

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After a relaunch, the first key a person types into the restored terminal can go to the sidebar list instead. On the feat tip (d3feb066), 10 of 15 Cmd-Q keep + reopen cycles on cmux21 lost the first key (REOPEN-F4, STATUS 4618). After a kill-all reopen, the line typed about 2 s after the terminals settled never reached the shell in 2 of 3 runs (REOPEN-F5).

The cause: the restored window comes forward before its focused terminal's surface is shown, and AppKit makes the sidebar list its first key view. The input journal of every lost cycle shows responder sidebar, programmatic 41-55 ms after the window became key. Then the focus stays on the sidebar (keyboard: false): the later contentPresented and restoredPane events do not move it back.

#30356 fixed this rule in cmux-focus-core: a sidebar responder the platform picks never takes a pane's keyboard. But the app links the reducer only through the pinned CCmuxAppFFI, and the pin (ae1499a) is older than #30356. The release cmux-app-ffi-382e54b7 exists (2026-11-01 07:40Z). Its bot repin #26434 stayed red because of base reds of that day (ActionCatalog counts, generated action surfaces, an xcstrings plural error), not because of the pin. So scripts/cmux-next/check-app-ffi-pin.sh has been red on every feat push since 382e54b.

This PR pins CCmuxAppFFI to cmux-app-ffi-382e54b7 (the change from #26434, on today's base) and adds the Swift tests that #30356 left for its PR 2. It supersedes #26434. check-app-ffi-pin.sh --verify-release passes at the head. The only FFI input change since the old pin is #30356 itself.

Why the dogfood line passes (lead question, STATUS 4872): it has its own fix of the same fault, 99c70f7 (perf/chrome-parity, 2026-10-27, ShellWindow.refusesOrderInKeyView: AppKit's first-key-view pick is refused while a window is ordered in). That commit was never ported to feat. Feat fixed the fault its own way in #30356, which this PR finally brings into the app. So no feat commit broke a working path: the feat app never had either fix.

The other half of REOPEN-F4 is a key typed before the focused terminal's surface exists (the panes show in hash order, one new surface per frame). That half goes in a second PR: the focused pane is presented first, and a key for a focused terminal with no surface yet waits for it (the 1193 hold).

Testing

Regression tests, WindowFocusRestoreTests (CmuxNextAppTests):

  • theSidebarAppKitPicksAsARestoredWindowComesForwardNeverTakesTheRestoredTerminal: the journal's order of a lost cycle (restored record, topology, window key, programmatic sidebar responder, contentPresented). The focus must stay on the restored terminal. A mouse click on the sidebar must still move the focus there.
  • aKillAllReopenWithAHomeColumnKeepsTheTerminalFocusedLongAfterItsSurfaceShows: the F5 shape (Home page column + one terminal). After the same events the focus and the published context must stay on the terminal.

Fleet runs (scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext with WindowFocusRestoreTests named 5 times, through proof-step.py). The refs carry the cmux-tui/Cargo.lock of base-red fix #31654 on top (proof only; the stale lock of #31462 stops every cmux-tui build on the base):

  • Green: job 4352d472a5ebfff4ada6211e at b683e9d (= head 124b668 + the lock file): 5 of 5 runs passed, all 3 tests each.
  • Red: job b25aa3d641b88dcecb28fe52 at 3d0a0a6 (= test commit e2b6fe3 on the old pin + the lock file): both new tests fail 5 of 5; the old test aRelaunchedWindowReturnsEachWorkspaceToItsFocusedPane passes 5 of 5.
  • End-result proof: journey reopen-first-key (kit ff80174; REOPEN VIDEO owner, STATUS 4951) GREEN 6 of 6 rounds on builds with this PR + fix(focus): a reopened window's focused terminal is presented first, and keys typed before its surface wait for it (REOPEN-F4) #31707 (30 Cmd-Q keep reopens, 60 marker keys each reached its terminal; 6 kill-all reopens, the typed line ran each time); base feat d3feb066 3 of 3 red.
  • CI cmux-next swift test red at the base-merge head adbd64a is a base finding: on the fleet, SidebarRenameInPlaceTests, NewTabHoldCancellationTests and SSHRecoveryShotsTests fail 3 of 3 at this head (job 15cb5f66ea83c91c8b30c23d) and 3 of 3 at its feat parent f6036b4 (job 3789d04de3a2f758a4aad1df); ToolbarButtonsStateTests passes 3 of 3 at both (a load timing in CI).

Observed / Not run

  • Observed: the 15 cycle journals in /work/tmp/reopen-video/rounds/r2-feat-marks-quit-keep-hid-light/ (kit cp/reopen-video-kit@d76e5b72c7a): lost iff responder sidebar, programmatic came before the key (10 of 15), kept iff the focused pane showed first (5 of 15). check-app-ffi-pin.sh --verify-release passes at the head. prepush-gate.py: no new violations.
  • Head compile (adbd64a): job 9b19b4c52b3df16c988d8e65 passed (CmuxNext, app, cmux-tui).

Changelog

Fixed: After cmux reopens, typing goes to the restored terminal instead of the sidebar

Proof

No capture: a focus routing fix with no visual change; the proof is the test runs and the reopen journey.

…d never takes the restored terminal (REOPEN-F4, red on the stale CCmuxAppFFI pin)

The Swift side of #30356: the app links cmux-focus-core only through the
pinned CCmuxAppFFI (ae1499a), which predates the rule. On the feat
tip, 10 of 15 Cmd-Q keep + reopen cycles on cmux21 lost the first key:
the journal shows 'responder sidebar, programmatic' 41-55 ms after the
window became key, and the focus then stayed on the sidebar.
…e): a sidebar AppKit picks never takes a restored terminal's keyboard (REOPEN-F4, REOPEN-F5)

check-app-ffi-pin.sh failed on every feat-cmux-next push since #30356
changed cmux-focus-core: app-ffi-release published cmux-app-ffi-382e54b7
(2026-11-01 07:40Z), and its bot repin #26434 stayed red on base reds of
that day. HEAD's FFI sources match 382e54b; --verify-release passes.

With the stale pin the app never got #30356's rule: at a relaunch AppKit
makes the sidebar list the first key view before the restored pane's
surface shows, and the focus stayed on the sidebar. The first key after a
Cmd-Q keep + reopen was lost in 10 of 15 cycles (REOPEN-F4), and the line
typed after a kill-all reopen never reached the shell (REOPEN-F5).
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 124b668: 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=pr31684 -f ref=fix/p0-reopen-f4-ffi-repin -f platform=macos -f backend_mode=local

Shipped in:

@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Found 2 test failures on Blacksmith runners:

Failures

Test View Logs
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)/
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (reopen-f4-reviewer-pr1): SHIP at 124b668

Independent correctness review. I read the code. I did not compile or run Swift or Rust on this machine. The fleet red/green results are pending; this verdict does not include them.

What I checked:

  • Pin: bash scripts/cmux-next/check-app-ffi-pin.sh --verify-release passes at the head ("sources match 382e54b", "release built from 382e54b with checksum 40d3608d..."). No other file in the repo refers to the old pin or the old checksum.
  • FFI input diff from ae1499a to 382e54b (all --list-inputs paths): 3 files, one commit, fix(focus): a sidebar responder the platform picks never takes a pane's keyboard (QUIT P0 second fault, Rust part) #30356. Only cmux-focus-core/src/reducer.rs changes logic (the new programmatic-sidebar branch in accepted). The other 2 files are Rust test data (focus/corpus.json) and Rust tests (focus_rules.rs). ae1499a is an ancestor of 382e54b.
  • Pin is not stale: no FFI input changed from 382e54b to the PR head, and none to the current feat-cmux-next tip (fbaa7a5).
  • ABI: no header, cmux_focus_* C ABI, wire, build script or toolchain file changed between the two releases. The Swift FocusMachine encode/decode agrees with the new binary as it did with the old one.
  • Commit order: e2b6fe3 adds only the tests, on the old pin. 124b668 changes only the URL and checksum.
  • Test 1 (theSidebarAppKitPicks...), by a trace of the reducer:
    • adopt(record) sends restoredPane(a2, ws-a). The topology then lands on a2 with target content. The first #expect is a precondition and passes on both pins.
    • Programmatic .sidebar responder: the old reducer gives Move(a2, sidebar{keyboard:false}), so resolved == .sidebar and the next three #expect calls fail. The new reducer gives Reapply (target is not sidebar, a2 is in the topology), so those three pass.
    • The mouse click at the end gives .sidebar on both pins. It is a guard against a block of a real click, not a discriminator. It passes for the correct reason on the new pin (Move with source mouse).
  • Test 2 (aKillAllReopen...): the old reducer stays on sidebar{keyboard:false} through contentPresented and appActive, so both #expect calls fail. The new reducer keeps terminal(term, term-tab). context.terminal comes only from Resolved::Terminal (state.rs context_of), so it is a real check that agrees with the first one and is not a vacuous pass.
  • Swift callers: no other Swift test or source path depends on the old behavior (a programmatic .sidebar report that takes focus). The two tests that make the list first responder use plain NSWindows and have no focus coordinator.
  • CI on this head, by class: cmux-next/compile is also red at the merge base 7a9e1ab and at the feat tip fbaa7a5. The fast tier fails with Cargo.lock ... --locked (this PR does not touch Cargo). tooling-tests fails on Developer ID and stub tests, release-notary on whats-new/test_digest.py, and cmux-next swift test is cancelled. I found no red that this PR causes.
  • Risk label risk:normal is correct: an FFI binary repin plus tests, focus logic, no auth, socket, release or signing.

Non-blocking notes:

  • PR body: after the fleet runs finish, add the exact focused command and its red result at e2b6fe3 and green result at the head. The "Testing" section says "results follow below" now.
  • The rule is fix(focus): a sidebar responder the platform picks never takes a pane's keyboard (QUIT P0 second fault, Rust part) #30356's and is out of this PR's scope: any programmatic .sidebar report while a pane has focus now bounces back to the pane. Example: an assistive technology or a future code path that makes the list first responder with no FocusTarget. A path that wants the sidebar must send FocusTarget first. I found no such Swift path today.
  • cmux-next/compile is a base red. It does not hold this PR, but a fleet compile of this head is still the real proof that the new binary links.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 03:34 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 03:34 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 03:37 Inactive
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Non-green checks at this head, classified (agent reopen-f4): tooling-tests (Developer ID and stub tests) and nightly-guards release-notary (whats-new/test_digest.py) fail in files this PR does not touch; the independent review (comment 6568309608) found them red on unrelated PRs too, so they are base findings. cmux-tui steps fail on the stale cmux-tui/Cargo.lock of #31462 (fix #31654/#31697). This PR changes only Package.swift's CCmuxAppFFI pin and one test file.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — adbd64af Deployed Nov 2, 2026 by cmux-bot[bot] via wait for same-key tree publisher #98939
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