Repository navigation
fix(app): a workspace asked for while the app connects opens in the launch window - #30080
Merged
Merged
Conversation
…launch window, and the first-launch Home landing leaves it shown (red) Dogfood gate v2.2.3, journey type (fails about 1 run in 8 on v2.2.2 and v2.2.3): newTab sent right after the app's socket answered made the workspace in the daemon, but the window stayed on Home and the typed keys went to the Chief composer. Ported from cp/fix-newtab-at-launch (0980a25 and the review tests of a7e5c3f), adapted to feat-cmux-next: feat's restore reads the saved windows from a live daemon, so the end-to-end case runs on the same-tree cmux-tui; the target, reconcile and quiet-create cases run with no daemon, before the restore.
…aunch window, and the first-launch Home landing leaves it shown Root cause (dogfood gate v2.2.3, journey type; also on v2.2.2, about 1 run in 8): until restore registers it, the launch window is outside the window registry, so New Workspace's target (targetWindow(preferring:)) was a new window id. The workspace was claimed for a second window, and restore's reconcile gave Home to the launch window and the new terminal to the other window. The keys went to the key window, Home's, and Enter sent them to the Chief. - targetWindow(preferring:) treats the launch window as open until the restore applied the saved registry (awaitsRestore), and a claim for it stays pending until the restore's reconcile places and selects it there: a reconcile before the restore (FirstTabHold's release, or the workspace's delta) would place it in a registry the restore replaces. - A create without focus at launch is a quiet claim in the launch window, as in any open window; it no longer sends the launch window behind. - adoptLaunchWindow: claims, sidebar rows and slots, a frame and a behind open asked for under the launch id follow the saved identity it takes. - FirstWorkspaceLaunch.land: its landing (Home) is decided before the restore places what was asked for during the launch; it no longer takes the window from a workspace of the person's own that came meanwhile. - A restore whose saved-window read failed for good (restoreGaveUp) places the held claims at once, so nothing waits for a restore that never comes. Ported from cp/fix-newtab-at-launch (9691f36, b9c0250 and the feat-relevant half of 7cd6360). Not ported: activeWindowID and b7e2a9b, which serve perf's launch picture without a controller; on feat the launch picture always has its controller (LaunchSnapshotWindow). feat's restore sets `restored` in the same turn as it applies the saved registry, so it stands for perf's savedRegistryApplied.
The launch-window claim fix adds App lines; this pays for them with a move of a pure value type out of CmuxNextApp (plans/cmux-next/code-quality.md SW14). PreviousLaunch (Restore Previous App Launch's plan) reads only ClosedItem (CmuxNextDaemon) and WindowRegistry (CmuxNextLayout); CmuxNextAppCore already had CmuxNextLayout in its closure through CmuxNextBridge and now declares it. `plan` is @mainactor because WindowRegistry's API is (CmuxNextLayout's default isolation); every caller is on the main actor. No behavior change.
…eKey (feat's typed keys)
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:22 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
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)
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 67aa3f8; does not block Shipped in:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift:101">
P3: This comment says no restore places the launch window's claims, which is the opposite of what the flag does. Say that the launch window's claims are no longer held for a restore.</violation>
<violation number="2" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift:255">
P2: A permanent saved-window read failure reconciles claims but never completes the window-restore lifecycle. `restored` and `registry.isLaunching` remain in their startup values, so post-restore observers and startup-dependent actions never begin; run the common completion/setup path or explicitly transition the manager into a usable degraded state before returning.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/FirstWorkspace.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/FirstWorkspace.swift:32">
P2: This startup guard only considers local-daemon workspaces, so a workspace created on a remote or Cloud machine can be missed. Include all machine stores when deciding whether the first-workspace landing is still needed, otherwise the launch landing can override the user's remote workspace.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| guard let document = await readSavedWindows(), !Task.isCancelled, !isTerminating else { return } | ||
| let saved = await readSavedWindows() | ||
| guard !Task.isCancelled, !isTerminating else { return } | ||
| guard let document = saved else { restoreGaveUp = true; return reconcileMembership() } |
There was a problem hiding this comment.
P2: A permanent saved-window read failure reconciles claims but never completes the window-restore lifecycle. restored and registry.isLaunching remain in their startup values, so post-restore observers and startup-dependent actions never begin; run the common completion/setup path or explicitly transition the manager into a usable degraded state before returning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift, line 255:
<comment>A permanent saved-window read failure reconciles claims but never completes the window-restore lifecycle. `restored` and `registry.isLaunching` remain in their startup values, so post-restore observers and startup-dependent actions never begin; run the common completion/setup path or explicitly transition the manager into a usable degraded state before returning.</comment>
<file context>
@@ -247,7 +250,9 @@ final class WindowManager {
- guard let document = await readSavedWindows(), !Task.isCancelled, !isTerminating else { return }
+ let saved = await readSavedWindows()
+ guard !Task.isCancelled, !isTerminating else { return }
+ guard let document = saved else { restoreGaveUp = true; return reconcileMembership() }
// Incognito workspaces a crashed run left on a daemon without state
// resources: the app's ledger owns them, so they close, never shown.
</file context>
| func land(_ workspaceID: String?, leftover: [String] = []) { | ||
| let services = manager.services | ||
| guard let workspaceID, services.machines.workspace(id: workspaceID) != nil, | ||
| services.daemon.store.workspaces.needsFirstWorkspace(leftover: leftover + [workspaceID], home: services.home.homeWorkspaceID) |
There was a problem hiding this comment.
P2: This startup guard only considers local-daemon workspaces, so a workspace created on a remote or Cloud machine can be missed. Include all machine stores when deciding whether the first-workspace landing is still needed, otherwise the launch landing can override the user's remote workspace.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/FirstWorkspace.swift, line 32:
<comment>This startup guard only considers local-daemon workspaces, so a workspace created on a remote or Cloud machine can be missed. Include all machine stores when deciding whether the first-workspace landing is still needed, otherwise the launch landing can override the user's remote workspace.</comment>
<file context>
@@ -24,9 +24,13 @@ struct FirstWorkspaceLaunch {
+ func land(_ workspaceID: String?, leftover: [String] = []) {
+ let services = manager.services
+ guard let workspaceID, services.machines.workspace(id: workspaceID) != nil,
+ services.daemon.store.workspaces.needsFirstWorkspace(leftover: leftover + [workspaceID], home: services.home.homeWorkspaceID)
+ else { return }
_ = manager.reveal(workspaceID: workspaceID)
</file context>
| /// Windows being closed by the registry (not by the user). | ||
| var programmaticCloses: Set<String> = [] | ||
| private(set) var restored = false | ||
| /// The saved windows' read failed for good: no restore places the launch window's claims (``awaitsRestore(_:)``). |
There was a problem hiding this comment.
P3: This comment says no restore places the launch window's claims, which is the opposite of what the flag does. Say that the launch window's claims are no longer held for a restore.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift, line 101:
<comment>This comment says no restore places the launch window's claims, which is the opposite of what the flag does. Say that the launch window's claims are no longer held for a restore.</comment>
<file context>
@@ -97,6 +98,8 @@ final class WindowManager {
/// Windows being closed by the registry (not by the user).
var programmaticCloses: Set<String> = []
private(set) var restored = false
+ /// The saved windows' read failed for good: no restore places the launch window's claims (``awaitsRestore(_:)``).
+ private(set) var restoreGaveUp = false
/// Windows the person closed before `restore` finished (cx-ghagp): the restore puts back the
</file context>
Suggested change
| /// The saved windows' read failed for good: no restore places the launch window's claims (``awaitsRestore(_:)``). | |
| /// The saved windows' read failed for good: the launch window's claims are no longer held for a restore (``awaitsRestore(_:)``). |
Contributor
Author
|
Review (claude-reviewer-30080): SHIP at ff46167 VerdictSHIP. I found no correctness defect. A claim that How I checked that a held claim always gets placed
What I ran (read-only)
Notes that do not block
|
lawrencecchen
added
the
ready-to-land
lawrencecchen
added
ready-to-land
|
Found 2 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Contributor
Author
|
Review (claude-reviewer-30080-merge): SHIP at 1fb24dd VerdictSHIP. The fix is still correct on the new base (feat-cmux-next 67aa3f8). I found no feat change that places workspaces, claims a window, or reads What I checked
The feat commits I read
PreviousLaunch move
Compile result I readFleet job fab5b25fc0380b7a8117cfd1 (cmux-next-swift-check v2, linux pool, ref 1fb24dd) is
I saw it pass. This is a compile check only. It ran no test. Coverage gaps
|
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A workspace asked for while the app is still connecting to cmux-tui now opens in the launch window, and the keys typed next reach its terminal. Before,
newTabsent at that point (thetypejourney, about 1 run in 8 on v2.2.2 and v2.2.3: 3/29 and 3/17) made the workspace in the daemon, but the window stayed on Home and Enter sent the typed command to the Chief composer.Takes back #29514: it does not undo #29514. PreviousLaunch.swift, which #29514 kept in CmuxNextApp, moves to CmuxNextAppCore with the same logic (only
publicand@MainActoronplanare added). This continues #29514's own aim of moving pure types out of the App, and pays for this fix's App lines.Summary
Until the restore registers it, the launch window is outside the window registry. So New Workspace's target (
WindowManager.targetWindow(preferring:)) was a new window id: the workspace was claimed for a second window, and the restore's reconcile gave Home to the launch window, the one the person sees.targetWindow(preferring:)treats the launch window as open until the restore applies the saved registry (awaitsRestore). A claim for it stays pending until the restore's own reconcile places and selects it there. A reconcile before the restore (FirstTabHold's release, the workspace's own delta) would place it in a registry the restore then replaces.adoptLaunchWindow: claims, sidebar rows, slots, a frame and a behind-open asked for under the launch id follow the saved window id it takes (launchWindowAdopted).FirstWorkspaceLaunch.land: the landing on Home is decided before the restore places what was asked for during the launch. It no longer takes the window from a workspace of the person's own that came meanwhile.restoreGaveUpthen places the held claims at once, so nothing waits for a restore that never comes.PreviousLaunch(a pure value type) moves from CmuxNextApp to CmuxNextAppCore. This pays for the fix's App lines: CmuxNextApp is 11 lines smaller after the PR.Ported from
cp/fix-newtab-at-launch(9691f36, b9c0250, and the feat-relevant half of 7cd6360), PERF EXTRACTION item 3. Not ported:activeWindowIDand b7e2a9b. They exist for the perf branch's launch picture, which has no window controller; on featLaunchSnapshotWindowalways builds the controller. The perf branch'ssavedRegistryAppliedis feat'srestored, which feat's restore sets in the same turn as it applies the saved registry.Testing
NewTabAtLaunchTests(CmuxNextAppTests), run on the fleet withCMUX_TEST_REQUIRE_GUI=1, so a skipped test fails the suite:perfx/newtab-at-launch-red: this head with the fix commit f57d733 reverted, never merge), aws-m4pro-1: FAILED, 4 of 4 tests on their own checks, 10 issues: the target is a second window (:60); a reconcile before the restore placed and dropped the claim (:81, :83); a quiet create opens a second window and sends it behind (:98-:100); end to end, another window shows the new workspace and the launch window shows none (:148-:157).cmux-ci compile-check --app --package CmuxNext --recipe-version 2) at ff46167: job 9d3fe078593f2335f75ce459 passed (CmuxNext, app, cmux-tui).The suite has five cases (the red ref has four: the adopted-id case comes with the fix): three with no daemon (New Workspace's target while connecting, a claim surviving the reconciles before the restore, a quiet create), one for the adopted id, and one end to end on the same-tree cmux-tui (
newTabsent once the connection exists and before the restore: one window, the launch window, showing the new workspace; the Home landing does not take it back).Observed / Not run
typejourney and a capture-lease run on a real key window. The perf branch's evidence for the same fix was new-tab-at-launch 21/21 PASS on fix builds 7809358c and c55aa3fc. That ran the perf code, not this port.Changelog
Fixed: A new workspace asked for while cmux is still starting opens in the window you see, and what you type goes to its terminal, not to the Chief
Proof
No capture: the change is in where a workspace is placed at launch, with no new UI; the tests above cover it.
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes a launch-window bug where a new workspace requested while the app is still connecting to cmux-tui opened in a separate window, leaving the visible window on Home and sending typed keys to the Chief composer.
targetWindow(preferring:)now treats the launch window as open until the restore applies the saved registry, and claims for it stay pending until the restore's reconcile places them.PreviousLaunchmoves toCmuxNextAppCore; addsNewTabAtLaunchTestscovering the target, reconcile survival, adopted identity, quiet create, and an end-to-end daemon case.Written for commit 1fb24dd. Summary will update on new commits.