Repository navigation
fix(cmux-next pages): a provider's state sent inside subscribe reaches the page - #27468
Conversation
…es the page (red) PageRouterEarlyEventsTests: a provider's state sent from inside subscribe reaches the page once, after the reply that names its subscription; an event after the reply goes to the page in its own turn. Both fail on feat-cmux-next 356f13f: #24617 inverted the guard in PageRouter.subscribe (fix in the next commit). Found on perf/chrome-parity, which has the same code (cp/fix-tip-page-script-batch-crash).
…s the page PageRouter.subscribe keeps the events a provider sends while subscribe runs (its current state) and flushes them once after the reply (20066e1: `guard early.isHolding else { release }`). #24617 (4abd404) rewrote that guard for providers that send only after the reply and inverted it: - events held during subscribe were released and thrown away: the first state of the agent activity, icon picker and update surface pages (AgentActivityPageProvider, IconPickerProvider, UpdateSurfaceIsland call onEvent inside subscribe) never reached the page, which showed nothing until its next change; - with nothing held, events after the reply were caught and sent a main turn late. The guard is now `guard early.isHolding, !provider.eventsAfterSubscribeReply`. Regression test: PageRouterEarlyEventsTests (red in the previous commit).
Bugbot needs on-demand usage enabledBugbot 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. |
|
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)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on e07b33b to compare; do not merge until green; see lane-rules Shipped in:
|
|
Found 6 test failures on Blacksmith runners: Failures
|
|
Bead |
|
Review (codex gpt-6-astra high fast, drain lane): SHIP at dfce161 No blocking findings. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
A page provider that sends its current state from inside
subscribelost that state. The agent activity, icon picker and update surface pages showed nothing until their next change.PageRouter.subscribeholds the events a provider sends whilesubscriberuns, and flushes them once after the reply that names the subscription (20066e1). #24617 (4abd404, "replay agent page events across bridge gaps") rewrote that guard for providers that send only after the reply (eventsAfterSubscribeReply). The rewrite inverted the guard:subscribewere released and thrown away.AgentActivityPageProvider,IconPickerProviderandUpdateSurfaceIslandcallonEventinsidesubscribe, so their first state never reached the page;The guard is now
guard early.isHolding, !provider.eventsAfterSubscribeReply. Nothing held: release at once. Held: flush once after the reply. Providers witheventsAfterSubscribeReply(the agent page) are unchanged.Thanks to #24617 for the bridge-gap replay; this restores the early-event path it changed. The same bug is on perf/chrome-parity and is fixed there in
cp/fix-tip-page-script-batch-crash(green proof 74b81ea13b25c872967366ac: PageRouterEarlyEventsTests, PageRouterFrozenDeliveryTests and PageScriptPipelineBatchTests pass).Changelog
Fixed: The agent activity, icon picker and update surface pages show their current state when they open, not only after their next change.
Testing
Regression test
PageRouterEarlyEventsTests(red commit 24e468d first, fix dfce161):cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg suite --arg Packages/macOS/CmuxNext --arg PageRouterEarlyEventsTests(throughproof-step.py):aStateSentInsideSubscribeReachesThePageAfterTheReplygot[]then[2](the held state was dropped);anEventAfterTheReplyGoesStraightToThePageInItsTurngot[].cmux-ci compile-check --app --package CmuxNext --recipe-version 2at dfce161 (fleet 21a81b1382d0303c8db082c1): passed.Observed / Not run
Observed: the two fleet runs and the compile above;
scripts/cmux-next/prepush-gate.pyclean.Not run: the full CmuxNextPagesTests target, and a live app check of the three pages in a tagged build.
Proof
No capture: the fix is in the page router's event delivery; no view or look changes. The fleet tests above are the proof.
Follow-up (review note, not in this PR): the throw and
closedpaths ofsubscribenever callearly.release(), so a provider that keepsonEventafter cancel would keep growing the held list. Fixed in #27500 (red 13 allocations over an empty loop for 10 000 events, green 0, in Debug and in a Release build).Review (independent subagent, correctness first): SHIP at dfce161
Checklist
PageRouterEarlyEventsTests)Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes the pages' initial state being dropped on open, so the agent activity, icon picker, and update surface pages now show their current state immediately instead of only after the next change.
A previous rewrite of the guard in
PageRouter.subscribeinverted it and discarded the events a provider sends whilesubscriberuns; that path is restored withguard early.isHolding, !provider.eventsAfterSubscribeReply, while the bridge-gap replay for providers that send after the reply is unchanged.PageRouterEarlyEventsTestscovering both the held-state flush and a post-reply event delivered in its own turn.Written for commit dfce161. Summary will update on new commits.