Skip to content

cmux-next: classic upgrade checks wait for placements to land, not for 0.5 s of quiet (#28425) - #28526

Merged
lawrencecchen merged 7 commits into
feat-cmux-nextfrom
fix/classic-sim-settle-signal
Oct 31, 2026
Merged

lawrencecchen merged 7 commits into
feat-cmux-nextfrom
fix/classic-sim-settle-signal

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #28425.

Cause

ClassicUpgradeSimulator.settle() (Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/ClassicUpgradeSimulator.swift:316-332 at base 7c41ce3) treated 0.5 s with no visible change as done. #28362 made it also wait for the personal placement queue, but a classic import places each new workspace at its slot through the sidebar document when the daemon serves sidebar-doc-v1 (a sidebar.apply move or sidebar.adopt sent from a task in SidebarDocStore.submit / adopt), not through PersonalPlacementQueue. Nothing told the test that those requests were still on their way, so on a loaded runner a late move or adopt changed the order after the quiet window. That is the failure on #28331 (anOffSSHHostDoesNotHoldBackLocalWorkspaces :239, aDowngradeAndReUpgradeMergesOnlyNewWork :184) and the ones #28425 lists.

The same gap was in the product: importClassicWorkspace waited for the personal queue before it wrote the workspace's classic group (#28362), but on the document path the slot move could still be in flight, and the daemon's document write-back of a late move takes the workspace out of its group again.

Fix

  • SidebarDocStore counts the changes and adopts it sent and not yet answered. isSettled (none in flight and no load running) and settled() (event-driven: each reply and load end resumes the waiters; no polling) are the completion signal.
  • The classic import waits for SidebarDocStore.current?.settled() after the personal queue, so the group write goes after the committed slot move on the document path too.
  • AppServices.placementsSettled() (the test helper that settle() and the ClassicMigrationTests launch helper use) waits for every completion signal at once: the personal queue through, the document settled, the store applied through an event barrier taken after those replies, no live workspace waiting for its new-workspace placement, and the document projection current (shownVersion == version). The 0.5 s quiet window is gone. A 60 s hang guard records an issue that names what is still in flight.
  • SidebarDocSettledTests: a move or an adopt keeps the store unsettled until its reply; settled() returns with the reply.
  • SidebarDocStore.swift shrinks: the remote protocol, its reply and SidebarDocUnsettled move unchanged to SidebarDocRemote.swift.

The known issue that stays (aRunCutOffMidwayFinishesWithoutDuplicates, "a resumed run makes its workspaces last in the sidebar") is a declared product limit from f9021c8, not the #28425 quarantine; #28445 already lifted that one.

Owners: sidebar store (CmuxNextSidebar) is a590 (sidebar-owner agent); onboarding/classic migration has no row in OWNERS.md (Performance Lead).

Performance

No perf numbers: import cost: in-process timing pending; not measured. The import now waits once per workspace for the reply to that workspace's own sidebar.apply slot move, which the old code already sent and did not wait for. Base and head cannot be paired on one fleet host, so two numbers from two hosts are not evidence. ClassicUpgradeSimulationPerfTests prints the per-workspace wait (CLASSIC-SIM-PERF-SETTLE, a DEBUG bench span in placementLanded) for a later run. perf claim: none.

Evidence

Both suites (ClassicUpgradeSimulationTests, ClassicMigrationTests) 10 times at the base and 10 times at the head, on the fleet (--pool perf-lead). The Mac proof steps are submitted under the alias branch fix/feat-classic-sim-settle (the same commit as this PR's head): the lead's fix/feat-* entry in the Mac-lane allow list covers feat base-red fixes (lead decision, 2026-10-30 21:0xZ).

Red evidence (base) is from CI, not from the fleet loop.

Base on the fleet did not reproduce it. Step d2dac36053cc5e74b8f9029b at base 7c41ce3 (cmux22-perf, an idle M6 perf host): 20 of 20 suite runs passed (10x ClassicUpgradeSimulationTests, 10x ClassicMigrationTests). There were 0 recorded issues and no gate-notice: FLAKY line; the only issues recorded were 11 known issues at :158, the declared resumed-run order limit. The race needs a loaded runner. So this step shows no base red, and the base-to-head comparison rests on the CI failures above.

First head, 7f80b7d (step 815e7963cc9c634d2d4e6762, aws-m4pro-4): order and group checks passed in all 20 runs, but ClassicUpgradeSimulationTests failed 5 of 10, all at :286 in backgroundWorkspacesStartTheirTerminalsWhenFirstShown ("scratch is not parked: [(false, nil)]"). park-tabs answers before the shells exit, and the removed 0.5 s window had covered that by chance. Fixed in 2dad1ec: the check waits for the tab-exit events (60 s hang ceiling). ClassicMigrationTests passed 10 of 10.

Forced-order proof, no clock (ClassicUpgradeHeldDocumentTests: every sidebar document change and adopt is held until someone waits in SidebarDocStore.settled(); the test reads, in the turn settle() returns, how many are still held):

  • Green, head 81c60ec (step b5cacafb3fd13ad8783e18ef, cmux17-perf): 25 of 25 suite runs passed: ClassicUpgradeHeldDocumentTests 5/5, ClassicUpgradeSimulationTests 10/10, ClassicMigrationTests 10/10. 0 recorded issues, no gate-notice: FLAKY; only the 15 declared known issues at :158.
  • Red, ref ffb9386 (head + base settle() with its 0.5 s window, base placementsSettled(), import without the document wait; branch never-merge/classic-sim-settle-red-v3): 5 of 5 runs red (step 9aed3d6aa9bafe68300e000d, cmuxs-Mac-mini-4.local): 4 failed at ClassicUpgradeHeldDocumentTests.swift:91 with settle() returned with 6 sidebar document changes still held (settled: false; 6 sidebar document changes or adopts unanswered, load running: false, ...); 1 stalled until the suite's time limit (198 s) with the moves still held. No run passed; no gate-notice: FLAKY.
  • CI cmux-next daemon tests on 81c60ec: pass (job 124333768628), the lane fix(next): Ctrl-`, Cmd-D and New Row always give a terminal; a refusal is never silent; no stand-in id reaches the daemon (P0) #28331 failed in.
  • Compile: fleet compile-check 40f2e41e45834805be58072a (CmuxNext, app, test targets) passed at 81c60ec.

Observed / Not run

Observed: the fleet steps above (job logs read in full, every attempt counted). The base loop did not reproduce the CI red. prepush-gate.py: no new violations. check-no-godfiles.sh --base: no file grew.
Not run: nothing compiled or tested on a local Mac. The full daemon-tests job runs in CI on this PR.

No capture: no UI change (a store completion signal, an import ordering wait and test helpers).

Changelog

none

…r 0.5 s of quiet (#28425)

ClassicUpgradeSimulator.settle() treated 0.5 s with no visible change as
done. Since #26153 a classic import places each new workspace at its slot
through the sidebar document (a `sidebar.apply` move, not the personal
placement queue that #28362 waits for), so on a loaded runner a move or an
adopt could still be on its way after the quiet window: the check then read
an order or a group the late move changed (#28331 attempts 1 and 2, #28425).

Product: SidebarDocStore counts the changes and adopts it sent and not yet
answered, and exposes isSettled and settled() (event-driven: each reply and
load end resumes the waiters). The classic import waits for it after the
personal queue, so a workspace's group write is sent after its slot move
is committed on the daemon, on the document path too.

Tests: settle() and the ClassicMigrationTests launch helper read the tree
only once every completion signal holds: the personal queue through, the
document settled, the store applied through an event barrier taken after
those replies, no live workspace still waiting for its new-workspace
placement, and the document projection current. No quiet window; a hang
guard records an issue naming what is still in flight.
SidebarDocSettledTests covers the new signal.

SidebarDocStore.swift: the remote protocol, its reply and SidebarDocUnsettled
move to SidebarDocRemote.swift (no change), so the file shrinks.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 81c60ec: 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=pr28526 -f ref=fix/classic-sim-settle-signal -f platform=macos -f backend_mode=local

Not verified: macOS jobs did not run: compile and app tests were skipped on 81c60ec7fe; ci-status is green without them.

CI passes on 81c60ec7fe (run 41155342616 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

Shipped in:

lawrencecchen and others added 2 commits October 30, 2026 21:13
…ent (#28425)

The import now waits for its slot move's reply before each group write.
ClassicUpgradeSimulationPerfTests records that wait per imported workspace
through a DEBUG bench span (BenchSpans, nothing recorded in release) and
prints n, sum and p50/p95/max per profile (CLASSIC-SIM-PERF-SETTLE). The
perf suite also holds the one SidebarDocStore (.ownsSidebarDocStore), as
the other live-daemon suites do.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or settle()'s quiet window (#28425)

backgroundWorkspacesStartTheirTerminalsWhenFirstShown read the parked state
right after settle(). `park-tabs` answers before the shells exit; a tab shows
dead with `relaunch` only once its host exited. The 0.5 s quiet window that
settle() no longer has covered that by chance; without it the last
workspace (scratch) read "not parked: [(false, nil)]" in 5 of 10 runs at
7f80b7d (step 815e7963). The check now reads once every background tab
shows parked (the daemon's tab-exit events), with a 60 s hang ceiling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 30, 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.

…held past any quiet window

ClassicUpgradeHeldDocumentTests holds every sidebar document change and
adopt (the classic import's slot moves) for 4 s before it reaches the
daemon, as a loaded runner's slow reply does. The check must read the tree
only once those moves have landed. The test seam is a
`wrapping:` parameter of SidebarDocWiring.start (default: the daemon remote
as is), passed by ClassicUpgradeSimulator(documentRemote:).

The red is on a scratch never-merge ref (this test over the base logic:
settle() with its 0.5 s quiet window and the import without the document
wait); this commit and the branch head are the green.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…waits for it, with no clock (#28425)

The 4 s hold was still a clock. HeldRemote now holds every sidebar document
change and adopt until someone waits in SidebarDocStore.settled() (the
store's onSettledWait test seam releases them; one sent while someone waits
goes on at once). The test reads, in the turn settle() returns in, how many
are still held: a settle that does not wait on the document returns with
moves held (red, every time); the new one cannot return until they are
released and answered (green). placementsSettled() now awaits the store's
settled() instead of polling isSettled.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…28425)

placementsSettled() keeps the event-driven SidebarDocStore.settled() wait,
with a 60 s hang guard (a ceiling, not a quiet window): if the store has
not settled by then, the recorded issue says what is pending, from the
store's new pendingDescription (changes and adopts unanswered, a running
load, changes a load must settle, waiters). The held-document test prints
it in its failure too.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…acement wait (#28425)

Compile 6af24f9b at 08910ed failed:
- ClassicUpgradeHeldDocumentTests.swift:73: the trailing closure matched
  ClassicUpgradeSimulator.init's `ssh:` parameter (String), not
  `documentRemote:`; it is passed by label now.
- AppServices+PlacementsSettled.swift:41,50: `pendingDescription` is
  internal to CmuxNextSidebar; the helper imports it @testable.

Also: the held remote's `waiting` is typed @mainactor, the local in
release() no longer shadows it, and after the read nothing more is held
(shutdown's changes go through). The await on the settled wait is its own
statement.

Sidebar-Look: no visual change
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Red checks on head 81c60ec, classified (none caused by this PR's changes):

Fleet proof (red/green with no clock) is in the description once the steps end.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (Claude Opus 5.5 reviewer): SHIP at 81c60ec
reviewer-session: 39921858-bd31-400c-8bf5-bd70588955ed
author-session: classic-sim-settle-author
R9: pass
perf claim: none

Scope: read-only review of 7c41ce3..81c60ec (7 commits, 11 files). I did not compile or run anything on a local Mac.

Correctness

  • The doc-path claim holds. applyPendingPlacements (WindowManager+Placement.swift:46-49) calls bridge.place, then then (the import's onListed). SidebarBridge.place (SidebarBridge+Windows.swift:40-41) goes into SidebarDocStore.move/submit in the same turn, so inFlight goes up before listed resolves. So settled() after queue.drained() (AppOnboardingServices+ClassicSessions.swift:131-133) covers the slot move.
  • The in-flight counter is balanced:
    • submit (SidebarDocStore.swift:393-396) increments after the last throw (the refusals at :379-387 throw before it). The one defer { self?.answered() } encloses the whole retry loop, so it decrements exactly once.
    • adopt (:244-248) works the same way.
    • When self is nil, nothing decrements, but then the store is gone. A caller in settled() keeps the store alive, so no waiter can lose its store.
    • settleByLoad calls refresh() before the defer runs, so loading takes over with no gap. The refresh defer (:166-170) resumes waiters after any re-fetch it starts.
    • Every change to inFlight or loading that can make the store settled calls resumeIfSettled. The while !isSettled loop in settled() (SidebarDocStore+Settled.swift:15-21) handles a new change that starts before the waiter runs.
  • Can settled() hang in the app? Not without an end. Each ask has requestTimeout (2 s default). There are at most unsettledAsks = 4 asks per change, and loads repeat a bounded number of times (unsettledAsks, or firstLoadAsks = 30 on the first load). notConnected goes to refresh(), which returns without loading when the remote is unavailable. Worst case: the import starts while the first sidebar.get keeps timing out, and the import can then wait about 30 × 2 s once. That is acceptable on an import that already awaits daemon replies.
  • Deadlock: none found. The import suspends, which frees the main actor. In-flight replies need only the daemon connection, not anything the import holds. settled() waits for the whole store (user drags, adopts and loads too), not only the import's own move. That is a longer wait, not a deadlock.
  • Main thread: nothing new blocks. The waits use withCheckedContinuation, and there is no sleep or .wait() in product code.

Rules

  • BenchSpans.mark(Self.documentSettledSpan, ...) (ClassicSessions.swift:132-135) is a measurement only. It records only under DEBUG || CMUX_BENCH_HOOKS, and only between begin() and end(). In release it reads the clock and does nothing more. It does not change timing or behavior. Small doc mismatch: BenchSpans says "main-thread spans", but this span is async wall time. mark()'s own doc allows a span timed somewhere else.
  • Test seams in product source (REVIEW.md §8 says no test seams):
    • SidebarDocWiring.start(..., wrapping:) (SidebarDocWiring.swift:34-38) is dependency injection with an identity default. It is fine.
    • onSettledWait and waitsForSettled (SidebarDocStore.swift:73, +Settled.swift:31) are hooks that only tests use. They are internal, nil in the app, and cost one nil check per wait.
    • pendingDescription and unansweredCount are internal diagnostics.
    • I accept them: cmux-next already has this pattern, and the hold test needs them to force the order with no clock. I do not block on them.
  • The test helper placementsSettled() (AppServices+PlacementsSettled.swift) has no quiet window, and its 60 s guard records an Issue that names what is pending. It still checks its other conditions every 20 ms (queue idle, appliedSequence, shownVersion). It waits for each condition, not for a period with no change, so this is acceptable. When the guard fires, settled(_:before:) leaves its waiter task behind. That happens only on the hang path, which already records the failure.

Discriminator

  • ClassicUpgradeHeldDocumentTests holds every document change and adopt until something waits in settled(). There is no clock. On the red ref ffb9386 (no import wait, old settle()), nothing calls settled(), so the held changes never go and stillHeld == 0 fails. If the run hangs instead, the 3 min limit fails it. On the head, the import and the helper release the changes. The helper releases the last group write.
  • Red step 9aed3d6aa9bafe68300e000d at ffb9386 was still running (compiling) at review time. Its result was pending.

Evidence checked

  • Fleet step b5cacafb3fd13ad8783e18ef at 81c60ec is done, exit 0. All 25 runs passed: ClassicUpgradeHeldDocumentTests 5x, ClassicUpgradeSimulationTests 10x, ClassicMigrationTests 10x. There was no failure and no FLAKY. The only issues were the known ones at ClassicUpgradeSimulationTests.swift:158.
  • CI cmux-next daemon tests job 124333768628 on 81c60ec: success.
  • The fleet compile-check 40f2e41e45834805be58072a reports done (I did not open it again).

Non-blocking

  • The PR body still says **Final head:** HEAD_COUNTS_PENDING. Put in the b5cacafb counts, and the red result when it comes.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 81c60ec7 Deployed Oct 30, 2026 by lawrencecchen via cmux-tui tree preflight #85260
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