Skip to content

fix(tabs): Cmd-W right after Cmd-T closes the new tab, not nothing (tip smoke red) - #31837

Merged
cmux-bot[bot] merged 3 commits into
feat-cmux-nextfrom
fix/cmdw-stale-standin
Nov 2, 2026
Merged

cmux-bot[bot] merged 3 commits into
feat-cmux-nextfrom
fix/cmdw-stale-standin

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Cmd-T, then Cmd-W soon after, sometimes closed nothing: the New Tab page stayed. This turned the tip smoke red at b732874 (STATUS 4852, step 66 "Cmd-W closes that tab").

Cmd-T shows the New Tab page as a stand-in tab (pending_tab_...). When the daemon's reply comes, the store replaces the stand-in with its own tab in one step. The pane's strip follows the store one observation step later. A Cmd-W in that step reads the strip's selected id, which is still the stand-in. PaneController.tab(_:) found no tab with that id, so Close Tab (and every other action on the selected tab) did nothing, with no message.

The fix: the store's intent log keeps, for the last 64 confirmed creations, which daemon surface each stand-in tab became. PaneController.tab(_:) resolves a stand-in id that the pane no longer lists to that tab while the pane lists it (DaemonCreations.replacement(ofStandIn:in:)). A stand-in that is still shown keeps the cancel path ("closed before created"); a cancelled or refused stand-in still resolves to nothing. CmuxNextApp lines: +0 (one line changed).

This race is old: it is not a commit in the tip range. The same failure shows at d15622d (2026-11-01) and on the last smoke-green build (7563ccd).

Testing

Regression test (committed red before the fix): CmdWOnReplacedStandInTests (CmuxNextAppTests, live same-tree cmux-tui). It shows a New Tab stand-in, lets the daemon make the tab (hidden behind the stand-in), runs the reply's turn as AgentTabStore.open does, and presses Cmd-W in that same turn, before the strip's observation step.

Fleet command (red at 9fe44fb, green at 2f4997e):
cmux-ci run --class light --script scripts/ci/package-test-lane.sh --ref SHA --arg=suite --arg=Packages/macOS/CmuxNext --arg=CmdWOnReplacedStandInTests,CloseTabKeepsPaneTests,RapidCloseTabTests,NewTabReplaceInPlaceTests --env CMUX_CI_XCODE_APP=/Applications/Xcode_26.6.app

  • Red on the base + test only (cf90261, branch fix/cmdw-stale-standin-red, fleet step 9c180fae, aws-m4pro-1): CmdWOnReplacedStandInTests failed at the Cmd-W check: "Cmd-W closed nothing: the pane holds [...], the New Tab tab is tab_6080d827...". The other 4 suites passed.
  • Green on the head (934d51b, fleet step dd7aae92, aws-m4pro-8): CmdWOnReplacedStandInTests, CloseTabKeepsPaneTests, RapidCloseTabTests, NewTabReplaceInPlaceTests and TabSpamRaceTests (it covers the intent log) all passed.
  • The first version of the test (9fe44fb) waited for the store to pair the daemon's raw tab-added with the stand-in. On the base run (step ed4e88f8) that wait timed out before the Cmd-W, so that run is no red evidence. 934d51b sets the same state without that wait.
  • scripts/cmux-next/prepush-gate.py: no new violations; arch ratchet: no count rises.

Observed / Not run

Observed:

  • Fleet diag journey (smoke journey to the browser page, then 16 Cmd-T/Cmd-W pairs 0-0.25 s apart, hog +15 GB, debug.key, cmux21): pair 2 (Cmd-W about 45 ms after Cmd-T) left the New Tab tab open on the tip build 84b90c53 (2 of 2 rounds) and on the last smoke-green build 45613f0e (1 of 1 round). Pair 1 (Cmd-W at once) closed the stand-in through the cancel path.
  • The tip smoke round's state: Cmd-W ran closeTab (handled_by app), no "closed before created" in new_tab_key_log, the page then reported ready at +134 ms and stayed; 25 s later the pane still had the New Tab tab selected.
  • Fix build 027c40e1 (2f4997e), diag round 1 (cmux24): the round failed earlier, at step 23 ("Cmd-D twice fast": both typed lines went to one new terminal), before the Cmd-T/Cmd-W pairs. That step has failed the same way on several tips before (Oct 31 tip-watcher rounds), and the fix does not touch the split path (.splitPane creations are not recorded). More rounds on this build are queued through the mini scheduler.

Not run: real HID Cmd-W rounds (the smoke's real-key rounds stop earlier at the New Column item, STATUS 1712/1724).

Changelog

Fixed: Cmd-W right after Cmd-T closes the new tab every time, instead of sometimes doing nothing.

Proof

No capture: no visible UI change; the evidence is the regression test and the fleet journey above.


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


Summary by cubic

Fixes a race where Cmd-W right after Cmd-T closed nothing, leaving the New Tab page open.

Cmd-T shows the New Tab page as a stand-in tab. When the daemon's reply arrives, the store replaces the stand-in with its own tab, but the pane's strip only catches up one observation step later. A Cmd-W in that step still targets the stand-in id, which the store no longer lists, so Close Tab silently found no tab.

The intent log now keeps, for the last 64 confirmed creations, which daemon surface each stand-in became. PaneController.tab(_:) resolves a stand-in id the pane no longer lists to that tab while the pane lists it. Cancelled or refused stand-ins still resolve to nothing.

Also adds a regression test that replays the racy turn and verifies Cmd-W closes the created tab.

Written for commit 934d51b. Summary will update on new commits.

View guided diff Turn on auto-fix

… closes the store's tab (red)

Tip smoke red at b732874 (STATUS 4852): Cmd-T on a browser page, then Cmd-W; the New Tab
page stayed. The strip shows the store one observation step late, so a Cmd-W in that step names
the stand-in (pending_tab_...) that the store already replaced, and Close Tab closes nothing.
Fleet diag (hog +15 GB, Cmd-W 30 ms after Cmd-T): fails on the tip 84b90c53 and on the last
smoke-green build 45613f0e (7563ccd). Red expected on the base: the tab stays.
…ses the store's tab (green)

The strip shows the store one observation step late. In the step after the daemon's reply
replaced Cmd-T's stand-in (pending_tab_...) with the store's tab, the strip still selected the
stand-in, the store no longer listed it, and PaneController.tab(_:) found nothing: Close Tab
(and every action on the selected tab) did nothing, silently. The intent log now keeps, for
the last 64 confirmed creations, which daemon surface each stand-in tab became, and
PaneController.tab(_:) resolves a stand-in id the pane no longer lists to that tab while the
pane lists it (DaemonCreations.replacement(ofStandIn:in:)). A cancelled or refused stand-in
still resolves to nothing. CmuxNextApp lines: +0 (one line changed).
…ing on the daemon's pairing

The first version waited for the store to pair the daemon's raw tab-added with the stand-in
(ProvisionalTab.matched); on the base run (step ed4e88f8, aws-m4pro-1) that wait timed out, so
the test failed before its Cmd-W. The daemon's tab is now listed first and the reply's turn
names it, which replaces the stand-in the same way; the Cmd-W assertion is unchanged.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@cursor

cursor Bot commented Nov 2, 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.

@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 934d51b: 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=pr31837 -f ref=fix/cmdw-stale-standin -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen lawrencecchen added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Nov 2, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (smoke-cmdw reviewer): SHIP at 934d51b

Non-blocking notes:

  • The test creates the daemon's tab before intend, so it shows beside the stand-in (production hides it via guess), and it sets pendingSelectSurface by hand instead of running open/tabCreated. It covers the close lookup, not the exact strip state. An #expect that the browser page is selected again would cover the second symptom.
  • The test is skipped without SameTreeDaemonBinary: please name the fleet run that shows red on base and green on head.
  • Strip-id consumers that bypass tab(_:) (cache.browsers[id] in livePageDecidesClose, pageClosing, the renameTab ask) still see the stale stand-in id for one step. For a frontend-browser stand-in, a Cmd-W in that step skips beforeunload. The New Tab page is not affected.
  • PaneController.swift:573 is now 160 characters.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 2, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
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.

@cmux-bot
cmux-bot Bot merged commit 70b28ba into feat-cmux-next Nov 2, 2026
264 of 293 checks passed
@cmux-bot
cmux-bot Bot deleted the fix/cmdw-stale-standin branch November 2, 2026 05:32
lawrencecchen added a commit that referenced this pull request Nov 2, 2026
…(follow-up #31837) (#31918)

* test(tabs): Cmd-W on a replaced browser stand-in asks the browser tab's page first (red)

Follow-up of #31837 (reviewer note, lead: data-loss risk). In the step after the reply replaced a
browser stand-in, Close Tab looks up the page that decides the close (beforeunload) by the strip's
id, the stand-in's, which no page has: the page is never asked. Red expected on #31837's head:
pageDecidesClose is asked about the stand-in id only.

* fix(tabs): Cmd-W on a replaced stand-in acts on its tab's id, so a browser page decides its close (green)

AppActionContext.tab(_:) returns the id of the tab PaneController.tab(_:) resolves (#31837) for the
strip's selected id: in the step after a reply replaced a stand-in, Close Tab, its beforeunload
check (CloseUndoToasts.livePageDecidesClose, cache.browsers) and pageClosing use the store tab's id,
not the stand-in's. Any other selected id is unchanged. CmuxNextApp lines: +0 (one line changed).

* test(tabs): Close Other Tabs in the replaced-stand-in step keeps the selected tab (red at 92644d5)

Review BLOCK of #31918 at 92644d5: with ctx.tab(_:) returning the store tab's id, the strip
(which still lists the stand-in) counted the stand-in as another tab, and its close resolved to the
tab the person kept.

* fix(tabs): only the close resolves a replaced stand-in's id; every strip-keyed action keeps the strip id (review BLOCK)

Takes back 92644d5's change to AppActionContext.tab(_:): about 25 actions key on the strip id
(Close Other Tabs, find, reload, ctx.terminal, multi-selection Cmd-W, empties/PaneCloseAim, close to
the right/left), and the strip does not list the store tab in that step. CloseUndoToasts.close(_:)
now maps each closing id to the tab PaneController.tab(_:) resolves after the pending-creation
filter, so the page that decides the close (livePageDecidesClose, cache.browsers, pageClosing) is
the store tab's: Cmd-W, the close button and the close commands. CmuxNextApp lines: +0.

---------

Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 934d51bf Deployed Nov 2, 2026 by lawrencecchen via namespace autofix #11958
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