Skip to content

perf(cmux CLI): workspace list reads the app window without the app's read barrier (CLI-1) - #32201

Merged
cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
perf/cli-list-no-app-barrier
Nov 2, 2026
Merged

cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
perf/cli-list-no-app-barrier

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Under the TYPING UNDER LOAD L6 load (STATUS 6075, cmux20), a CLI workspace list took up to 4,173 ms (8 workers); with 32 workers (video-extra-2's base self-check) p50 903 ms and a tight cluster at 4.08-4.36 s.

Cause: after the daemon answers workspace list (or show, rename, move), the CLI asks the app which workspace its window shows (snapshot.get, the focused overlay, cli/app_focus.rs). Every CLI app request carries the read barrier after: "sync" by default (cli/app.rs with_read_barrier). The app answers it only after a MainActor.run hop, an identify round trip on its one daemon connection, and a wait for the main actor to publish a snapshot past that sequence, within its 2 s deadline (ControlRouter+Barrier.swift, AppControl+Barrier.swift). Under load the deadline passes, and the CLI asks once more (is_retryable_read): 2 s + 2 s = the 4.1 s cluster. A list waited for the app's main thread, against the rule "a list call is a read, it must never wait behind the main thread".

Fix: the overlay first reads the app's last published snapshot (after: 0), which needs no daemon round trip (the storm's plain snapshot.get took 2.5 ms p50 under the same load). The app may still refresh a stale snapshot on its main actor before it answers (fresh()), as before. Focus goes through the app's focus actions, which answer after the window moved, so a focus is in that snapshot. One daemon write can leave the snapshot behind: closing the workspace the window shows. When a workspace list of the app's session lacks the window's workspace, the overlay reads the window once more with after: "sync".

Not changed: every other CLI app request keeps after: "sync".

Owners: the cmux CLI (cmux-tui/crates/cmux-tui, no OWNERS row) and the app control router (PL-socket / app control, not changed here).

Changelog

  • Fixed: cmux workspace list no longer waits seconds for a busy app window under load.

Testing

Regression test: workspace_list_reads_the_window_without_waiting_for_the_apps_read_barrier (cmux-tui, cli/app_focus/tests.rs). A fake app answers a request with after: "sync" with its own deadline error (a busy main actor) and any other request with the window snapshot. The list must read the window in one request and set focused from it.

  • Red at 71e905a (test only): step 2aead6b11fc5501ea879bc28 FAILED as expected: the CLI sent 2 snapshot.get calls, both after: "sync" (the first timed out, the retry timed out), so focused stayed the daemon's.
  • Green and full cmux-tui suite at 8185ff0: step f574cdf961d469f5334df65f: 15,231 of 15,234 passed, the new test among them. The failures are not in files this PR touches: acpmux usage_stats::the_session_list_and_info_carry_what_the_chats_transcript_says (base red on every head today), cmux-tui every_safe_transport_operation_has_a_noun_first_path (workspace_group.create has catalog fields with no exercised CLI representation: a base red of feat 706ef7f, the catalog op came from another PR), and acpmux subsessions_safety (passed when rerun alone: a load flake).

Command: cmux-ci run --class isolated --script scripts/ci/cmux-tui-rust-check.sh --ref SHA --os linux --arg test --arg workspace_list_reads_the_window_without

Review fix (first review SHIP at 8185ff0 with a Medium: after workspace close <shown>; workspace list, the fast snapshot can still show the closed workspace): when a list of the app's session lacks the window's workspace, the overlay reads the window once more with after: "sync". Test a_list_without_the_window_s_workspace_reads_the_window_again_with_the_barrier.

  • Red at fe82728: step 0b0f825ef5a8431668352fd5 FAILED as expected (only one after: 0 read was sent).
  • Green at 82312c5: step 3c90020eb58a5a13f5a46cf6, app_focus and agents tests 14 of 14.

Observed / Not run

Observed: the raw L6 calls (8 and 32 workers), the code path, the red and green steps above.
Not run: an L6 round in the real app with this CLI. The A/B needs the app under load; the measurement owner (video-extra-2) has the daemon-stats capture ready for the next L6 rounds.


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


Summary by cubic

Fixes cmux workspace list waiting seconds for a busy app window under load. The list asked the app which workspace its window shows (snapshot.get) with the default read barrier after: "sync", which waits for the app's main thread; under load each attempt hit the app's 2 s deadline and the CLI asked twice, so a list took up to 4.2 s.

The overlay now reads the app's last published snapshot (after: 0), answered off the main actor. Which workspace a window shows is the app's own state, and no daemon write of the caller changes it. Other CLI app requests keep after: "sync".

Testing

  • Adds a regression test with a fake app whose read barrier never settles, verifying the list reads the window in one request and sets focused from it.

Written for commit 8185ff0. Summary will update on new commits.

View guided diff Turn on auto-fix

… read barrier (CLI-1)

Under the TYPING UNDER LOAD L6 load a CLI `workspace list` took up to
4.2 s (32 workers: p50 903 ms, a cluster at 4.08-4.36 s). After the
daemon answered, the CLI asked the app which workspace its window shows
(`snapshot.get`, the `focused` overlay) with the default read barrier
`after: "sync"`: a daemon round trip on the app's connection, then a wait
for the app's main actor to publish a snapshot past it, within the app's
2 s deadline. Under load the deadline passed and the CLI asked once more.
Which workspace a window shows is the app's own state; no daemon write
of the caller changes it (focus goes through the app's focus actions,
which answer after the window moved). The overlay now reads the last
published snapshot (`after: 0`), answered off the main actor.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@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 8185ff0: 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=pr32201 -f ref=perf/cli-list-no-app-barrier -f platform=macos -f backend_mode=local

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 8185ff0

Scope: cmux-tui/crates/cmux-tui/src/cli/app_focus.rs (app_workspaces sends snapshot.get {"after": 0}) and the new test in cli/app_focus/tests.rs. I read the code and used git only. I did not compile or run tests.

What the app does with after: 0

  • ControlRouter.handle (ControlRouter.swift:262-274) first runs snapshots.fresh(), then, because after is not null, readBarrier. .number(0) gives sequence 0 (ControlRouter+Barrier.swift:19-20). There is no daemon identify round trip and no ControlDeadline hop. snapshot(reflecting: 0) (ControlSnapshot.swift:139-182) returns the current snapshot under the store lock when topology.isLoaded is true (reflects, line 37). snapshot.get is a .snapshot builtin (ControlRouter+Builtins.swift:94), so it does not use the work queue. When the topology is loaded, the reply comes at once and does not wait for the main actor. The one exception is below.
  • Before the topology loads, reflects(0) is false. The request waits until the 2 s requestDeadline and then fails with timeout. request (app.rs:1141-1147) sends it one more time because snapshot.get is a retryable read. That gives about 4 s, and then app_workspaces returns None, so there is no overlay. The old "sync" path waits the same way after syncBarrier is registered. In the short window before that, it failed at once with unavailable. This applies only at app launch, so it is only slightly worse than before.
  • Older apps: the numeric after arrived in the same commit as "sync" (e348cfc, case .number ... value >= 0 && value == value.rounded()). Every app that accepted the old CLI's "sync" accepts 0. with_read_barrier uses entry().or_insert, so the explicit 0 is kept.

Findings

  1. Medium (follow-up, does not block): the claim "no daemon write of this caller changes it" is false for closing the shown workspace. Code: app_focus.rs:132-139 and overlay_focused app_focus.rs:211-214. Take the script cmux workspace close <the workspace the window shows>; cmux workspace list. The daemon list no longer has the closed workspace C. The app's last published snapshot can still be from before the close echo. Daemon echoes only call invalidate() for the next frame, and fresh() rebuilds only after a control work-queue frame. So that snapshot has shown = C, and known holds every other id. overlay_focused then sets focused: false on every row, so the list reports no focused workspace until the app applies the echo and publishes. Under load, which is exactly when this PR matters, that window is longer. The old "sync" barrier waited for a snapshot whose daemonSequence covered the close. I did not confirm that the window leaves C in the same main-actor turn as the echo, so the old path may also have been briefly wrong. The new path is wrong for longer. A cheap fix that keeps the fast path: in after_daemon_with, for Follow::Focused, when app.shown is in app.known but missing from the daemon reply's workspace ids on a WorkspaceList, the fast snapshot is older than a close. In that case read once more with the default barrier. Add a test with a fake app that returns a pre-close snapshot to after: 0 and a post-close snapshot to "sync".

  2. Low: "off the main actor" is true only when the snapshot is not stale. Code: app_focus.rs:139, PR body. fresh() (ControlSnapshot.swift:102-105) runs before the barrier and is not bounded by the request deadline. It awaits the main-actor publishIfStale after any control work-queue frame (ControlSnapshotPublisher.swift:88-92). This is not new; the old path did the same. But under a load that also runs app action.run calls, a list can still wait for the main actor once per stale mark. The same mechanism gives the read-your-writes for finding 3, so do not remove it. Correct the comment wording. The L6 A/B is still "Not run", as the PR says.

  3. OK: workspace focus X then workspace list. focus_in_app runs goToWorkspace through action.run with wait. PaletteController.swift:116-128 calls selectWorkspace → SidebarNavigation.activate / windows.show, which is synchronous on the main actor. The work-queue frame then runs afterFrame → markStale() (MainActorWorkQueue.swift:340-358, ControlSnapshotPublisher.swift:88-92). The next snapshot.get rebuilds in fresh() and sees the new window workspace, whatever the after value is. In theory, a second CLI process could reach fresh() before the hook runs, because the hook runs after the rest of the frame's items. That needs a process spawn plus a daemon round trip inside one frame drain, and the old barrier did not protect against it either, because goToWorkspace does not write to the daemon.

  4. OK: workspace create / workspace <new> show. The CLI workspace create never moves the daemon focus. The router defaults focus: false (resource_router/topology.rs:305-308, 345-350), and command.rs:3309 accepts --no-focus as a no-op. WorkspaceCreate is Follow::Nothing. If the fast snapshot does not have the new workspace yet, overlay_focused leaves that row as the daemon says (false, which is correct). For workspace <new> show, no id in the reply is in known, so it returns early and keeps the daemon's false. That is correct because the window does not show it. rename and move (reorder by --index, command.rs:708-713) do not change the shown workspace.

  5. OK: other callers. app_workspaces has one caller only: after_daemon_with (app_focus.rs:255). agents.rs:594-598 calls overlay_focused with AppWorkspaces::showing(..) built from its own snapshot read, which keeps the default barrier. It does not change.

  6. Low (test): the test is behavioral and fails on the base for the right reason. On the base: the first request gets "sync" → timeout, the retry gets the same, so received.len() == 2, and the first assertion fails with the fleet red described in the PR. The focused assertions would also fail, because app_workspaces returns None and the reply stays as the daemon's. One nit: assert_ne!(received[0]["params"]["after"], "sync") also passes when after is missing or has any other value. assert_eq!(received[0]["params"]["after"], 0) states the contract. There is no vacuous assertion: the reply is changed from the daemon's (ws_new focused) to the app's (ws_home).

  7. Low (REVIEW.md "Docs and comments out of date"): the with_read_barrier doc comment is now wrong for this case. Code: app.rs:1050-1054, which says every app request first lets the app catch up. It now has an exception. Name it there, or name it in plans/cmux-next/state-ownership.md 4.3. Optional: after: null (the router skips the barrier when after is null) would avoid the 2 x 2 s wait before the topology loads. An unloaded topology has no focus.workspace, so the overlay would be skipped at once.

REVIEW.md: §1 (tests): passes, apart from the nit in 6. §2 (stale results): finding 1. §6 (budgets): unchanged, CLI READ_TIMEOUT 5 s against the app's 2 s. Smaller groups (docs): finding 7. The PR has the risk:normal label, a ## Changelog line, red/green steps and an ### Observed / Not run section.

@cmux-bot
cmux-bot Bot merged commit 3983c49 into feat-cmux-next Nov 2, 2026
184 of 200 checks passed
@cmux-bot
cmux-bot Bot deleted the perf/cli-list-no-app-barrier branch November 2, 2026 11:40
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at 82312c5

Scope: the whole diff origin/feat-cmux-next...82312c5f64a (cli/app.rs, cli/app_focus.rs, cli/app_focus/tests.rs), with focus on the review fix fe82728 + 82312c5 (window_left_the_list). I read the code and used git only. I did not compile or run tests. Note: the GitHub PR API still reported head 8185ff0 when I checked, but origin/perf/cli-list-no-app-barrier is at 82312c5.

The fix answers the earlier Medium

after_daemon_with (app_focus.rs:277-282) reads the window with after: 0. If window_left_the_list is true, it reads again with after: "sync" and overlays from that second read. window_left_the_list (app_focus.rs:162-174) is true only when (a) the reply is a JSON array, or an object whose value is an array, (b) no item's id equals app.shown, and (c) at least one item's id is in app.known. For workspace close <shown>; workspace list, the daemon list lacks C, the stale snapshot has shown = C, and the other rows are known, so it fires. The sync snapshot covers the close, and the overlay uses it. The new test checks exactly this sequence: 0, then "sync", and focused comes from the second read. It fails on fe82728 for the right reason (one request only), and none of its assertions is vacuous.

Reply shapes (from resource_router/topology.rs and cli/wire.rs:620-631)

wire.rs passes the daemon's response.result unchanged to app_focus::after_daemon. That is the typed, non-stream path only.

  • WorkspaceList → list_workspaces (topology/workspace_order.rs:12-51): a bare Value::Array of every workspace of the session (public_workspaces), sorted by order. There is no filter and no pagination, so a list never omits the shown workspace for a legitimate reason when that workspace is in the session. The check works as intended.
  • WorkspaceGet → find_snapshot(...): one workspace object, with no value key. Result: false, no second read.
  • WorkspaceRename / WorkspaceMove → snapshot_mutation_result → mutation_result (resource_router.rs:925-938): {"value": {object}, "generation", "revision", "replayed"}. value is an object, not an array. Result: false, no second read.
  • Empty list: ours stays false. Result: false.
  • known empty (an app that sends no workspaces): false, as before.

So the common case (the window's workspace is in the list) costs one fast read only. busy_app still sees exactly one request.

Findings

  1. Low (performance, not a regression): the second read fires on every list while the focused window shows a workspace of another session. Code: app_focus.rs:162-174. ControlTopology.workspaces includes the workspaces of the remote sessions (ControlSnapshotPublisher.swift:195-204), and focus.workspace is the active window's workspace from any session. If the person views a remote machine's workspace while an agent runs cmux workspace list against the home daemon, then shown is never in the list but the home rows are in known. Each list then pays fast read + "sync" read. That is about the base cost (the base always paid "sync"), so the PR's speedup is lost only in that case, and the overlay result is correct (every home row is unfocused). The same happens for a list from a remote daemon while the window shows a home workspace. A refinement for a follow-up: the app's workspace JSON has session (null for home), and daemon rows have session_id. Read again only when the shown workspace belongs to the listed session (for example, app.shown's app record has the same session as a matched row). Does not block.

  2. Low (stale result, narrow): single-record replies get no second read. workspace close <shown>; workspace <X> show|rename|move: if the app moves the window to X after the close, the stale snapshot (shown = C, X in known) makes overlay_focused set X.focused = false until the app publishes. window_left_the_list cannot see this from one record. The window is short, and the next workspace list repairs it. A follow-up could read again when app.shown is not a workspace the daemon reply could confirm. Does not block.

  3. OK: failure of the second read. app_workspaces(stream, "sync") returns None on transport error, app error or timeout (request retries one timeout, so the worst case is about 2 × 2 s, the same as the base path). Then if let Some(app) skips the overlay, and the reply keeps the daemon's focused values. That is the base behaviour on failure. It is the better choice here, because the stale fast snapshot would mark every row unfocused. A socket-level READ_TIMEOUT leaves a late reply on the stream, but the stream is dropped right after, so nothing reads it.

  4. Low (REVIEW.md "Docs and comments out of date"): two places still state the contract that the fix disproved.

    • cli/app_focus/tests.rs:213-215, the doc of workspace_list_reads_the_window_without_waiting_for_the_apps_read_barrier: "which no daemon write of the caller changes ... never waits for the main actor". The new test right below it shows that a close does change it, and fresh() can still await the main actor (earlier finding 2).
    • The PR body's "Fix" paragraph says the same ("no daemon write of the caller changes it", "answers off the main actor"), and so does the cubic summary. The new code comment at app_focus.rs:267-276 is correct; make the test doc and the PR body match it.
  5. Earlier review points. Finding 1 (Medium): fixed for lists, see above, with residuals 1 and 2. Finding 2 (comment wording): fixed in code, still in the PR body (see 4). Finding 3, 4, 5: still hold; the new code does not change focus, create or agents.rs. agents.rs still calls overlay_focused only, and app_workspaces still has one caller. Finding 6 (test nit): fixed (assert_eq!(..., 0)). Finding 7 (with_read_barrier doc): fixed (app.rs:1054-1056). with_read_barrier keeps an explicit after (or_insert), so both 0 and "sync" reach the app as sent.

REVIEW.md: §1 tests: behavioral, red on fe82728, green on 82312c5, no vacuous assertion. §2 stale results: residual 2 (Low). §3 empty input: the empty list and empty known are handled. §6 budgets: worst case unchanged against base. Smaller groups (docs): finding 4. The label risk:normal is correct for a CLI read path. The body has red/green steps and ### Observed / Not run. The L6 A/B in the real app is still "Not run", as the PR says.

@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
cmux-bot Bot pushed a commit that referenced this pull request Nov 2, 2026
…dow again with the barrier (#32201 follow-up) (#32327)

* test(cmux CLI): a list without the window's workspace reads the window again with the barrier (#32201 review red)

* fix(cmux CLI): a list that lacks the window's workspace reads the window again with the barrier (#32201 review)

The review of #32201 found one daemon write that can leave the app's
last published snapshot behind: `workspace close <shown>` and then
`workspace list`. The fast snapshot still shows the closed workspace, so
the overlay set every row unfocused. When a list of the app's session
lacks the window's workspace, the overlay now reads the window once more
with `after: "sync"`; every other list keeps the fast read. The read
barrier's doc names the caller-chosen `after`.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 8185ff03 Deployed Nov 2, 2026 by lawrencecchen via cmux-tui tree preflight #100813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant