Reset a browser pane's selected frame and element refs when the page navigates

Merged
NE
7d ago
+438-57

Summary

browser frame select stores a frame selector for the surface, and nothing cleared it when the page changed. After a navigation to a page without that frame, the eval wrapper couldn't find the selector and quietly used the new page's top document, so eval, click and the rest reported success against the wrong document. Element refs (@e1, ...) had the same lifetime problem: a ref from the old page stayed resolvable and replayed its selector against the new one.

Now a main-frame commit ends both for that surface:

  • commands go back to the main frame, as after browser frame main
  • refs handed out before the commit fail with not_found: Element reference not found, and their ordinals are never reused

The commit signal is the panel's existing WKNavigationDelegate didCommit callback, so this covers browser navigate, reload, back and forward, and also a link the page follows or a redirect the user triggers. A fragment change or history.pushState doesn't commit, so a frame selected in a single-page app stays selected. Other surfaces aren't touched.

One consequence to know about: cmux also reloads a page on its own, when it restores a discarded hidden WebView, recovers from a web content crash or switches profile. Those commit too, so they reset the frame. Before this change the selector happened to match again on the reloaded page. I kept one rule for every new document instead of special-casing these, since page state is lost in all three anyway.

browser state load used to write the saved frame_selector into the map before it navigated. It now restores it last, after the page has loaded and storage is applied, and only if the selector still names a same-origin frame in that page (the same probe frame select runs). Otherwise the surface stays in the main frame. A navigation is reported at commit, before the page has parsed its iframes, so this probe waits for DOMContentLoaded first, for at most two seconds. A failed load no longer changes the frame selection at all.

frame select checks its selector with a round trip to the page. If the page commits a new document during that check, the command fails with stale_state, whatever the check found, instead of answering for a page that is gone.

Mechanically: the selected frame and the refs were three dictionaries on TerminalController. They move into one value, BrowserAutomationDocumentState in CmuxBrowser, which the controller still owns, so the rule "a commit drops this surface's entries" can be tested without the app. BrowserPanel posts browserMainFrameDidCommit from didCommit, and the controller's observer drops the state synchronously on the main thread, before any later socket command can read it. BrowserStateLoadTransaction gains a restoreFrameSelection step.

No socket method, parameter or response field changes, and the relay policy is untouched.

Fixes #16131

Testing

  • 83f03834be4 moves the state into CmuxBrowser with no behavior change. 21de9eb2902 adds the failing regressions, plus mainFrameDidCommit as an empty hook so they compile. 8cc28ee3508 is the fix. ce88fbe44ba and c753ff6cd73 apply two subagent reviews: the parse wait in state load, bounded to two seconds in the page, and the stale_state check in frame select. d7141602547 applies CodeRabbit's point that every probe outcome should report stale_state after a commit; the app was rebuilt with it.
  • swift test --package-path Packages/macOS/CmuxBrowser --filter 'BrowserAutomationDocumentStateTests|BrowserStateLoadTransactionTests' at 21de9eb2902: 5 of 10 tests fail with 8 issues. The selected frame and the ref both survive mainFrameDidCommit, the frame selection is restored before cookies and navigation, and it is restored even when cookies, navigation or storage fail. At 8cc28ee3508 all 10 pass, and 11 at c753ff6cd73 with the replaced-document case.
  • The full CmuxBrowser package suite passes (731 tests).
  • CMUX_DEV_BACKEND_MODE=local ./scripts/reload.sh --tag baseline --build-only compiled the app with the change. python3 scripts/verify-local.py and git diff --check pass.
  • Not run: anything against a live WebView. The tests cover the state rule and the load ordering. The wiring from didCommit to the controller and the frame probe script are compiled only, and I didn't launch a tagged build to run the issue's five steps. tests_v2/test_browser_api_extended_families.py exercises frame.select over the socket but needs a running app.

Changelog

Fixed: After a browser pane navigates, automation commands no longer keep running against a frame selected on the previous page, and element refs from that page are rejected instead of acting on the new one

Proof

No UI change. The red and green runs above are the evidence; runtime behavior is unverified as noted.

Checklist

  • Behavior changes have added or updated tests, or Testing says why not
  • UI, settings, menu, schema, help-text or user-facing docs change: localization audited, and the result is stated above (no new or changed strings)
  • New or changed v2 socket method allowlisted for cmux ssh: the relay authorization questions are answered above (no method added or allowlisted)
  • User-facing docs updated if needed (none needed)
  • Reviewed with a subagent before merge (cmux-review), and all bot and human review comments resolved

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


Summary by cubic

Resets a browser surface's selected frame and element refs when its main frame commits a new document, so automation commands no longer silently run against the wrong page after a navigation.

Previously, a frame selected on one page outlived that page: after a navigation the stored selector matched nothing and eval and click quietly ran against the new page's top document, and element refs from the old page replayed their selectors on the new one. Now any main-frame commit (browser navigate, reload, back/forward, a followed link, a redirect) returns the surface to the main frame and makes its refs fail with not_found: Element reference not found; ref ordinals are never reused. A fragment change or history.pushState doesn't commit, so a frame selected in a single-page app stays selected. Note that cmux's own reloads — restoring a discarded WebView, recovering from a crash, switching profiles — also reset the frame, since page state is lost in all those cases anyway.

Behavior details

  • browser.state.load restores the saved frame selector last, only when the load succeeded and the selector still names a same-origin frame in the loaded page, waiting up to two seconds for the page to parse.
  • browser.frame.select refuses a selection with stale_state whenever the page commits a new document during its own-frame probe, regardless of what the probe found.
  • State moved into BrowserAutomationDocumentState in CmuxBrowser; the panel posts browserMainFrameDidCommit from didCommit, and the controller drops the surface's state synchronously before any later command can read it.

Fixes #16131.


[!NOTE]
Medium Risk
Changes browser automation command semantics and state-restore ordering; incorrect timing could still cause stale selectors, but behavior is tightened and covered by new unit tests.

Overview
Main-frame navigation now clears per-surface automation context so browser frame select and @e* refs cannot keep acting on a new document after the old page is gone.

Frame selectors and element refs move from ad-hoc dictionaries on TerminalController into BrowserAutomationDocumentState (CmuxBrowser). On each main-frame commit, BrowserPanel posts browserMainFrameDidCommit; the controller drops that surface’s selected frame and refs synchronously on the main thread (same-document navigations like fragments/pushState do not commit, so SPA frame selection is unchanged).

browser.frame.select records a document generation before probing the frame; if the page commits during the probe, selection fails with stale_state instead of storing a selector for the wrong document. browser.state.load applies saved frame_selector only after cookies, navigation commit, and storage—probing for a same-origin frame (with a bounded parse wait)—and skips frame restore on any failed step.

Reviewed by Cursor Bugbot for commit d7141602547b2b29608899faafe1a1f53fbcf984. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Frame selections and element references stay scoped to the browser surface and document where they were created, and are invalidated when that document changes.
    • Restored browser state returns to the main frame unless a saved frame selector still identifies a same-origin frame in the restored document.
    • Frame selection rejects stale selections when navigation changes the document before selection completes.
    • Saved frame selection is restored only after cookies, navigation when requested, and storage have been applied successfully.