Restore interaction state in legacy browser fallback - #15795

Merge Risk: 🔵 Low · up to b2ac7

A browser may reopen with developer tools visible after they were just closed, and delayed focus can leave keyboard intent on the wrong panel. The fallback-state test also needs stronger coverage. These bounded issues merit follow-up before relying on the restoration behavior.

Architecture Summary

Architecture risk: 🔵 Low · up to b2ac7

The change affects 2 systems.

Changed systems: Sources, cmuxTests

Architecture concerns
No architecture-level concerns identified.

❌ Failed checks (2 errors, 1 inconclusive)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Thanks for opening your first cmux pull request!

We're a small team and the outside-PR queue is long, so a reply can take a while, sometimes longer than we'd like. If this one goes quiet and you'd like eyes on it, comment here and we'll pick it up.

A few things that help:

• Start here covers what reviewers look for, what CI runs for you, and what happens next.
• If the CLA check asks, reply with the sentence it gives you.
• The verification ladder shows which checks fit your change. Say in the description which ones you ran.
• If we end up fixing the same problem another way, we'll credit you with a Co-authored-by trailer and link the fix here.

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Thanks @Tiancheng-Xu, the approach is right :) It deletes ClosedBrowserPanelRestoreSnapshot, but TabManagerUnitTests.swift (4193, 4217) and BrowserEphemeralRestorationTests.swift (177) still use it, so the test target won't compile. Keep the typealias or update those three to .fallbackSnapshot. Then mark it ready and sign the CLA, and we'll run CI.

Updated the three legacy test references to LegacyClosedBrowserPanelRestoreSnapshot; the placement assertions now read through .fallbackSnapshot. Both edited test files pass Swift syntax-only parsing. I moved the PR to Ready, but type-check/XCTest still need the upstream run because this machine has Command Line Tools but no full Xcode. The CLA check is still failing; I need to post the repository-required signature statement myself.

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

I have read the CLA Document v2.2 and I hereby sign the CLA

Taking this: reviewing the legacy browser restoration repair and its updated test references against main.

• OrchardSpoon g1 🌀

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

Gates only: only gate jobs failed; see the run.

CI failed on 87d8f3daa7 (run 38253687487 attempt 1): no failed job besides the gates.
macOS jobs did not run: compile and app tests were skipped: changes missing, Fast static checks missing.

Not re-run automatically: only gate jobs failed.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Quick update: I pushed follow-up fixes at e714d84 for the review findings: reconcile a stable attached-inspector manual close before capturing the session snapshot while preserving detach/reattach intent; reject stale focus callbacks before they record keyboard focus intent; and strengthen the forced legacy-fallback test around the compatibility snapshot. The PR description now distinguishes syntax-only parsing from XCTest. Current-head checks are green where run, but no native XCTest/build job appears in the run list and I could not run XCTest locally. Could you re-review this head and let me know whether the previous changes request is satisfied or anything remains?

Thank you @Tiancheng-Xu, both fixes look right to me. However, DockSplitStore+SessionSnapshot.swift:180 still reads the preferred visibility without consuming the pending manual close first, so the dock path has the same bug. Could you apply the same reconcile there? CI is approved and running on this head.

CI fast guards failed on e714d84f97 (https://github.com/manaflow-ai/cmux/actions/runs/37167091395). It does not block the merge; a red guard merged into main breaks it for every open PR.

Validate macOS jobs select a pinned Xcode

Test: python3 tests/test_ci_macos_xcode_selection.py

ci.yml job dogfood-build never runs scripts/select-ci-xcode.sh, so it builds with the image's default Xcode

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate macOS jobs select a pinned Xcode'

Validate fork runner routing

Test: python3 tests/test_ci_fork_runner_routing.py

• ForkRunnerRoutingTests.test_every_fork_exercised_runner_has_a_hosted_path (workflow='ci.yml', line=1722)

AssertionError: False is not true : ci.yml:1722 has no GitHub-hosted macOS fork branch

• ForkRunnerRoutingTests.test_fork_pull_requests_into_manaflow_ai_never_reach_an_owned_runner_label

AssertionError: Lists differ: ["ci.yml:1722 checks vars.MACOS_RUNNER_PR [101 chars] }}"] != []

First list contains 1 additional elements.
First extra element 0:
"ci.yml:1722 checks vars.MACOS_RUNNER_PR before the fork pull-request branch: runs-on: ${{ vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-26' }}"

+ []
- ['ci.yml:1722 checks vars.MACOS_RUNNER_PR before the fork pull-request branch: '
-  "runs-on: ${{ vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-26' }}"]

• ForkRunnerRoutingTests.test_no_workflow_falls_back_to_blacksmith_outside_manaflow_ai

AssertionError: Lists differ: ["ci.yml:1722: blacksmith-6vcpu-macos-26 i[192 chars] }}"] != []

First list contains 1 additional elements.
First extra element 0:
"ci.yml:1722: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}"

+ []
- ['ci.yml:1722: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, '
-  'where no Blacksmith runner exists; start the expression with the owner fork '
-  "branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' "
-  '|| ... }}'] : 
ci.yml:1722: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate fork runner routing'

Validate owned Mac build state

Test: python3 tests/test_ci_owned_build_state.py

• E2EWiring.test_an_unexpected_root_reads_nothing (root='/private/tmp/cmux-ci-x')

AssertionError: 0 == 0

• E2EWiring.test_each_root_reads_its_own_state_and_the_macs_packages (root=None)

AssertionError: 1 != 0 : bash: line 6: CMUX_CI_CANONICAL_ROOT: unbound variable

• Wiring.test_a_second_compile_slot_keeps_its_own_root_and_state

AssertionError: Tuples differ: (0, 'CMUX_CI_CANONICAL_ROOT=/private/tmp/cmux-[32 chars]i\n') != (0, '', 'root=/private/tmp/cmux-ci\n')

First differing element 1:
'CMUX_CI_CANONICAL_ROOT=/private/tmp/cmux-ci\n'
''

- (0,
-  'CMUX_CI_CANONICAL_ROOT=/private/tmp/cmux-ci\n',
-  'root=/private/tmp/cmux-ci\n')
+ (0, '', 'root=/private/tmp/cmux-ci\n')
? +++++++

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate owned Mac build state'

Run canonical CMUX CI guard profile

test_only_mac_work_waits_for_static_preflight

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Run canonical CMUX CI guard profile'

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally. This comment is updated in place on each push.

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Status from a maintainer pass, @Tiancheng-Xu. I tried to apply the Dock fix Leo asked for, but I couldn't prove it, so I've reverted my commits. The branch is now your code plus a merge of main (87d8f3daa7).

What I tried: the same consume-before-read as your Workspace fix, in DockSplitStore+SessionSnapshot.swift:180. The test was a Dock browser's closedPanelSessionSnapshot after a pending attached-inspector close.

Why it didn't work: the test still read developerToolsVisible == true with the fix in place. The last state dump was pref=1 vis=0 attached=1 presentation=attached tx=1. Hosting the panel in a main window and starting it attached didn't help. Spinning the run loop for the 0.15 s settle didn't clear the transition (tx=1) either. So consumeAttachedDeveloperToolsManualCloseIfNeeded bails on isDeveloperToolsTransitionInFlight before it ever reaches the snapshot.

Note for whoever picks this up: a Dock browser's DevTools start detached (preferredDeveloperToolsPresentation defaults to .detached). That matters for any Dock test.

The original request still stands: apply the same reconcile on the Dock path, with a test that fails without it. If you have a way to settle the transition in a test, that's the missing piece.