Repository navigation
cmux-next: new splits no longer vanish when made in quick succession - #26926
austinywang wants to merge 14 commits into
Conversation
…anishing (#26695) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heck (#26695) (red) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sionally never vanishes (#26695) (red) split-vanish-live.py attaches to a tagged app, presses Cmd-D back to back (debug.key, no read-your-writes wait) and fires split action bursts, and fails when the shown pane count goes down or settles short. Red on ffa3b4c (tag 26695-splits-vanish): cmd-d presses settle at 3-4 of 5 panes (view 2,3,4,2,3), cmd-d then browser split at 2 of 3; the app logs 'split rejected: split failed: unknown pane 46116860184273879xx'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e daemon's pane (#26695) A second Cmd-D (or a browser split, New Tab page split, new tab) pressed on the pane a first Cmd-D still showed provisionally sent that stand-in's client-only handle (>= 2^62). The daemon refused it ('unknown pane'), the intent log rolled the split back, and the new pane showed, then vanished. New Column and New Row resolved the target at send time but behind their own lane; Cmd-D, the New Tab page split, browser tabs and new terminal tabs did not. DaemonStore.daemonPane(confirming:) waits, event-driven (runs after every overlay apply, drains on disconnect), until the daemon's pane takes the stand-in over under its public id, or the stand-in's create is refused. Every create aimed at a pane goes through it: PaneSplitCommand.target (Cmd-D, New Column, New Row, new terminal tab), the browser tab open funnel and its split, and the agent / New Tab page tab create. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…batches run its waiters (#26695 review) Review findings on the first cut: - An event batch applies under applyDepth, so its lifts skipped the stand-in waiters and the flush ran them only on a disconnect: a create aimed at a stand-in could wait until an unrelated lift. flushAppliedWaiters now runs them after every batch. - A target resolved after an await (browser split after its tab open, a new terminal tab after its cwd read) no longer knew the stand-in once its split settled, and was refused. DaemonStore.paneTarget(_:) names it in the press's turn (PaneTarget); daemonPane(confirming:) resolves that later. - Cmd-N's first pane is a stand-in too: PaneTarget.workspace waits for the daemon's workspace to bring its pane. - The browser split opens its tab in the daemon's pane, then splits it. Tests: InstantCreateIntentTests createOnAPendingSplitNamesTheDaemonsPaneOnceItsBatchLands and createOnARefusedSplitGetsNoPane (they name PaneTarget, so they do not compile on the base). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mputed subscript (crash ratchet) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…live check reads the open window (#26695) RapidSplitsKeepPanesTests passed on the pre-fix commit (step c5c3e1b79cb77fb8c0444a27): its 30 ms gap let the daemon confirm each split before the next press, so no press hit a stand-in. Presses now follow each other with a yield only, as fast key presses do. split-vanish-live.py took debug.windows' first entry, which after a relaunch can be a saved, closed window; it reads the first open one now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Bead |
Review (correctness subagent): SHIP at 1140f97Earlier round, on intermediate heads (all fixed before this head):
This round, on the final head. Nothing blocks; three LOW notes:
Verified independently by the author: the base and head live checks, and the red and green fleet runs listed in the description. |
|
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)
|
Default changesThis pull request changes 5 defaults at
Needs a review. It passes with an independent review receipt on this commit: a comment or review whose first line is Regenerate the snapshot with |
…its end on cancel (#26695) Review findings on #26926: - A .createTab aimed at a split's stand-in resolves through the stand-in (as the split overlay does) and keeps its target in standInTargets, so Cmd-T right after Cmd-D no longer vanishes until its echo; the conversation and browser tab guesses match the resolved pane too. - daemonPane(confirming:) ends with nil when its task is cancelled, and a stand-in whose create was cancelled resolves to no pane rather than the pane about to close. - A stand-in that never got its pane throws StandInPaneGone (not "the daemon refused the split"), and the browser split refusal names the public pane id, not the raw stand-in handle. - Tests wait until the waiter is queued, so they drive the queued path; new tests for a tab on a pending split, a cancelled wait and a cancelled split. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…26695) The overlay resolved only split stand-ins to the daemon's pane; Cmd-N's first pane has no public pane id, so a Cmd-T or Cmd-D pressed on it before its workspace's first tab landed vanished for a round trip. Its workspace key is remembered (standInWorkspaces) while a create targets it, and daemonHandle(showing:) resolves it to the daemon workspace's first pane, the pane the send side already names (firstDaemonPane). Stand-in targets are released after every apply, not only while split panes are pending. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review (correctness subagent): SHIP at e545b3c PR: #26926 · author austinywang · agent Claude Opus 5.5 · base feat-cmux-next 8e36e52 · head e545b3c Three correctness-first subagent passes, findings and resolution: Pass 1 at 4b9a87e (FIX)
Pass 2 at 4983fb3 (SHIP) (4983fb3 = a test-only compile fix:
Pass 3 at e545b3c (SHIP), on the delta from 4983fb3
Checked: compile and access levels, waiters resume exactly once (including cancel before and after enqueue), ordinary daemon panes unchanged (no wait, no log scan), overlay order and undo, no tab shown twice, Fleet |
The prepush gate refuses an unowned Task in service code; the onCancel hop to the main actor only ends its own waiter, which the store's waiter list owns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review (correctness subagent at e545b3c; comment-only delta checked by the primary agent): SHIP at e57e3b1 PR: #26926 · author austinywang · agent Claude Opus 5.5 · base feat-cmux-next · head e57e3b1 e57e3b1 only adds a @@ -58,6 +58,8 @@ extension DaemonStore {
standInWaiters.append(StandInWaiter(id: id, target: target) { done.resume(returning: $0) })
}
} onCancel: {
+ // task-owner: a one-shot hop to the main actor (onCancel runs on any thread) that ends
+ // this waiter; the store's waiter list owns it, and a waiter already run is a no-op.
Task { @MainActor in self.dropStandInWaiter(id) }
}
}The findings, their resolution and the subagent SHIP for the code are in #26926 (comment). The fleet step d45637dd0710eab15a281ade (6/6 suites passed) ran at e545b3c, which has the same code apart from this comment. |
# Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/NewColumnCommand.swift
|
Sweep: blocker was a merge conflict with feat-cmux-next (1332 behind). Merged origin/feat-cmux-next at f8bd6ac (new head 71c3157). Resolved Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/NewColumnCommand.swift: base renamed the call to newColumn(beside:left:width:options:) for New Column Left; kept that plus this PR's async stand-in target (try await target()). This head needs CI and a review receipt before relabel. |
|
Removed ready-to-land: this PR conflicts with feat-cmux-next (DIRTY), so no merge can take it. Merge feat-cmux-next into the branch, resolve, and get a new receipt at the new head; then add the label again. |
|
For awareness: this PR conflicts with feat-cmux-next, so it can't land until it's rebased or base is merged in. Its current review verdict stays as is; a hand-resolved merge gets a fresh review at the new head. |
Summary
In cmux NEXT, a split or browser pane made quickly after another split showed up and then vanished (#26695, also reported as #26689).
Cmd-D shows its new pane at once as a provisional pane and focuses it. That pane's handle exists only in the app (≥ 2^62) until the daemon's pane takes it over under the same public id. A second Cmd-D, a browser split, a New Tab page split or a new tab pressed in that window sent the provisional handle to the daemon. The daemon refused it (
split failed: unknown pane 4611686018427387960in the app log), and the intent log rolled the create back, so the pane disappeared. New Column and New Row already resolved their target when sending. Cmd-D, browser tabs, agent/New Tab page tabs and new terminal tabs did not.Fix:
DaemonStore.daemonPane(confirming:)returns the daemon's pane behind a stand-in, so every create aimed at a pane names that pane. When the daemon has already taken the stand-in over, it answers at once. Otherwise it waits, event-driven, without polling: its waiters run after each overlay apply and are released when the connection ends. It returns nil when the stand-in's own create is refused, and the dependent create fails the same way it does today. Callers:PaneSplitCommand.target(Cmd-D on both paths, New Column, New Row, new terminal tab), the browser tab open funnel and Split Browser's move, and the agent / New Tab page tab create.Review follow-ups (#26926 (comment)): a new tab pressed onto a pending split's pane, or onto Cmd-N's first pane, now stays shown in the daemon's pane instead of vanishing for a round trip; a cancelled wait sends nothing; a create aimed at a cancelled split gets no pane.
Not in this PR: the late-reply path, where a split whose reply misses its 5 s start deadline on a loaded host is rejected even though the daemon commits it. It needs a keyed retry and is a separate follow-up.
Testing
Regression check:
scripts/cmux-next/split-vanish-live.py SOCKET OUT_DIR PID. It attaches to a tagged DEV app, presses Cmd-D back to back throughdebug.keyand fires split action bursts. It fails when the shown pane count goes down or settles short.ffa3b4cdd6e0(fleet jobabc473a11c7aeb03f2c717f0, tag26695-splits-vanish): FAIL.cmd-d pressessettles at 3 or 4 of 5 panes (view counts2,3,4,2,3), andcmd-d then browser splitsettles at 2 of 3.1140f975d19a9live check (fleet builde09c6c2096f6be8f262c5551at56928e56, same app code): PASS 15/15 phases over 3 rounds. Every Cmd-D burst settles at 5 panes;cmd-d then browser splitsettles at 3 (2 terminals + browser), counts never go down.Unit regression (
Tests/CmuxNextAppTests/RapidSplitsKeepPanesTests.swift, back-to-back presses with no wait between them), run on the fleet:7ccd183e91765(base fix-free code + this test only): FAIL,.terminalSplitsand.mixedbursts atRapidSplitsKeepPanesTests.swift:54(Expectation failed).e545b3cde7f137cb7b5a3d638a821d68ecce34b1(stepd45637dd0710eab15a281ade): PASS, 6/6 suites:RapidSplitsKeepPanesTests,InstantCreateIntentTests(new: stand-in waits queued, refused, cancelled; a tab on a pending split and on Cmd-N's pane),NewTabIntentTableTests,NewTabSplitRefusalTests,SplitBrowserFailureTests,DragSendsOneIntentTests. Superseded:1140f975d19a9(step035383c28ffb7abe87e91e5f),4983fb35(stepf2df026b33103173b92668ec).Observed / Not run
BrowserStandInIntentTestspassed at56928e56;scripts/cmux-next/prepush-gate.pyclean.e545b3cd: the live check (last at56928e56); the review follow-ups change only the overlay and the stand-in wait, covered by the unit runs above.0a7c1b94c669band5b671f6b0eca4(theRapidSplitsKeepPanesTestssuite andrapid-split-keeps-panes-e2e.py) came from another session working the same issue; I kept and tightened the test.Changelog
Fixed: New splits and browser panes made in quick succession no longer appear and then vanish
Proof
No capture: no visible UI changes, only panes that no longer disappear. The live check's window snapshots were taken on a local isolated DEV app and are not posted because they show the local account; the pane counts above come from them.
Checklist
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes new splits, browser panes, tabs, and Cmd-N's first pane vanishing when created in quick succession (#26695): a create pressed while an earlier one still shows its pane provisionally sent the provisional pane's client-only handle to the daemon, which refused it and rolled the create back.
Creates now name their target in the press's turn (
DaemonStore.paneTarget(_:)) and resolve it before sending.daemonPane(confirming:)waits event-driven for the daemon's pane to take the stand-in over, or returns nil when the stand-in's create was refused, cancelled, or its waiting task is cancelled. A tab pressed onto a pending split stays shown in the daemon's pane; Cmd-N's first pane resolves through its workspace key. Applies toPaneSplitCommand.target(Cmd-D, New Column, New Row, new terminal tab), the browser tab open funnel and Split Browser's move, and agent / New Tab page tab creation.Testing
RapidSplitsKeepPanesTestspresses splits back to back with no gap;InstantCreateIntentTestscovers the wait, refusal, cancellation, tab-on-pending-split, and Cmd-N pane paths.scripts/cmux-next/split-vanish-live.py(and the same surface viarapid-split-keeps-panes-e2e.py) fails on the pre-fix base, settling at 3–4 of 5 panes, and passes on head.Written for commit 71c3157. Summary will update on new commits.