Skip to content

cmux-tui: a sidebar group collapse answers while busy sessions hold the registry - #34565

Merged
teamleaderleo merged 1 commit into
feat-cmux-nextfrom
lane/tui-group-update-latency
Nov 4, 2026
Merged

teamleaderleo merged 1 commit into
feat-cmux-nextfrom
lane/tui-group-update-latency

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Leo, on the sidebar: "what's up with this ? im just clicking the expand/collapse situation in the workspaces is kinda awk" / "as in im doing it but it sends this message for no reason". The notice read: "Collapse/Expand Workspace Group did not apply: update-personal-group: timed out: workspace_group.update (no reply within 2.0 seconds)". workspace.place and new-conversation-tab (cmux agent open) timed out the same way on the same Mac.

Context: manaflow-ai/cmuxterm-hq#6338 (comment 6597127823). #34405 (merged) hides the notice by retrying; this fixes why the daemon was slow to answer. A sibling change handles the notices themselves; this PR touches no UI and adds no Swift.

Cause

workspace_group.update is one short SQLite commit (under 1 ms idle). It commits under the one workspace registry lock (SignaledMutex<WorkspaceRegistry>), which every state write takes: on a busy Mac (about 50 agent sessions writing status, progress and logs) the lock is held most of the time. std::sync::Mutex is not fair: a waiter can lose to newcomers again and again, so the click's commit queued behind the whole crowd of background writers until the app's 2 s deadline. Persistence is not the problem: commits already reply before the disk (wal_durability.rs: synchronous=NORMAL, the syncer flushes off the request path), and the commit itself is unchanged here.

Fix

  • mux/signaled_mutex.rs: a request a person waits on runs inside interactive_scope(). Its registry locks go next after the current holder: while one waits, a plain locker neither queues for the lock nor keeps one it just won.
  • Bounded fairness: at most MAX_INTERACTIVE_STREAK (4) interactive acquisitions go ahead of waiting plain lockers in a row; then interactive lockers step aside the same way until a plain one has taken the lock. Agents send personal writes too (workspace.place, group edits), so this keeps them from starving ordinary writers (status updates, workspace create/close).
  • state/personal.rs (state_personal: every personal sidebar write: group create/update/move/delete, workspace.place, rooms) and state/conversation_tabs.rs (new_conversation_tab_as) run in that scope.

No new timer or loop at idle: a plain locker waits on the existing release condvar only while an interactive locker waits (20 ms timeout bounds a missed wakeup); nothing runs when nobody waits.

Trade-off and risk

  • Durability: unchanged. Same transaction, journal record and WAL commit, replied after commit as before; nothing is deferred or coalesced, so a crash right after the reply loses nothing it did not lose before.
  • Ordering: a person's write can now go ahead of up to the queued background writers; each is still one serialized transaction, so no write is lost or reordered within itself. Background writers wait at most 4 interactive holds in a row.
  • Risk: lock-order and fairness code on the registry lock every state write takes. A bug here could deadlock or starve writers; covered by the existing state, signaled_mutex, registry_reads, personal and agent-session-tab suites (259 tests) and the starvation test below.

Tests (red at base, green at head)

cargo test -p cmux-tui-core --lib group_update_latency_tests (run with cmux-ci test, set -o pipefail):

  • a_persons_requests_answer_while_busy_sessions_saturate_the_registry: 48 sessions commit workspace_status.set back to back (each commit holds the registry 5 ms), 4 cmux agent open callers open chat tabs, and a person collapses a group, moves a workspace and opens a chat tab 30 times each. Budget p95 < 100 ms for the sidebar writes, < 200 ms for the chat tab; the last fold must be in the registry file after a restart. Red at base (9f536b8: 480 / 494 / 2659 ms p95), green at head in 3 of 3 rounds (04ad745 and again at the final head 3e02bae, which only reorders a use for rustfmt).
  • ordinary_registry_writers_progress_while_personal_writes_stay_queued: 48 sessions send workspace_group.update back to back with no pause; an ordinary workspace_status.set must still answer (p95 < 1 s). Passes at base and head; it guards the starvation follow-up from review.

The chat tab costs about 9 ms idle but takes the registry lock several times (browser row, presentation reload, topology commit) and shares interactive turns with the 4 agent-open callers, so it answers in about 65 ms p50 / 100 ms p95 under this load; its budget is 200 ms (10x under the app's 2 s deadline), and fewer registry takes per tab is a follow-up.

Performance

Same harness (group_update_latency_tests, fast-checks Linux 8 vCPU, test profile), 48 busy sessions, 4 agent-open callers. Before: base code (run 41762350362, at 9f536b8). After: final head 04ad745, 3 rounds (run 41774733365); ranges across rounds.

metric before after delta harness
workspace_group.update p50 224.9 ms 6.4-10.6 ms -95% person_requests test
workspace_group.update p95 480.2 ms 25.4-26.7 ms -94% person_requests test
workspace_group.update max 490.9 ms 26.2-30.0 ms -94% person_requests test
workspace.place p50 248.4 ms 4.7-5.3 ms -98% person_requests test
workspace.place p95 493.7 ms 17.4-26.4 ms -95% person_requests test
workspace.place max 517.4 ms 19.5-26.7 ms -95% person_requests test
new-conversation-tab p50 1980.1 ms 65.6-77.7 ms -96% person_requests test
new-conversation-tab p95 2659.0 ms 100.1-116.2 ms -96% person_requests test
new-conversation-tab max 2801.6 ms 108.8-122.1 ms -96% person_requests test
ordinary workspace_status.set p95 (48 personal writers) 269.9 ms 7.1-7.5 ms -97% ordinary_writers test
idle new-conversation-tab p95 - 11.9-15.5 ms - person_requests test, idle phase
idle workspace_group.update p95 - 0.8-1.3 ms - person_requests test, idle phase

Background status throughput during the measured window falls (the person's requests and the agent-open callers go first), which is the intended trade.

Observed / Not run

  • Observed: the red run at base and the green runs at head above; at 04ad745 cargo clippy -p cmux-tui-core --all-targets -- -D warnings is clean and the neighbor suites (state::, mux::tests::signaled_mutex, mux::tests::registry_reads, server::personal, server::agent_session_tabs: 259) pass.
  • Observed at the final head 3e02bae (run 41778776271): both latency tests green 3 of 3, 811 cmux-tui-core lib tests pass, clippy clean. The full workspace Rust check's reds (fmt in acpmux, clippy in acpmux/cmux-tui/cmux-wg-tun, tests in acpmux, cmux-agent-pane-policy and cmux-tui) are in files and tests this PR does not touch; compared by name against base.
  • Not run: a live repro on a busy Mac in a DEV build.

Changelog

Fixed: Collapsing or expanding a workspace group, moving a workspace and opening a chat tab answer quickly while many agents are busy.

Review: Codex (gpt-6.1-sol xhigh) range reviews, SHIP at 3e02bae; receipt in a comment.

Agent session: agent session

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

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

Review c9c6c68: 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=pr34565 -f ref=lane/tui-group-update-latency -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Base-red classifier (per test) (+1 more) is not red on the base 291f606; do not merge until green; see lane-rules

Shipped in:

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp6qmzbmj9/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmppm1xq4ww/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol xhigh, router-review range 427dd19..3e02bae in deltas): SHIP at 3e02bae

  • 427dd19..ade9aa5: SHIP, one [P2 follow-up] (interactive promotion could starve ordinary writers). Addressed in this PR: bounded streak with handoff (f0509dc) plus ordinary_registry_writers_progress_while_personal_writes_stay_queued.
  • 427dd19..5ef016b: BLOCK (doc comment on thread_local! trips clippy; streak cap not enforced as a handoff). Both fixed in f0509dc.
  • 427dd19..f0509dc: SHIP, no findings.
  • f0509dc..05a500d (test budget for the chat tab): SHIP, no findings.
  • 05a500d..04ad745 (clippy qualifications, is_multiple_of): SHIP, no findings.
  • 04ad745..3e02bae (rustfmt use order in mux.rs): SHIP, no findings.

Visual verdict: unchanged by this diff (no UI code).
Tests at 3e02bae (fast-checks run 41778776271; also 41774733365 at 04ad745): cargo clippy -p cmux-tui-core --all-targets -D warnings clean; 259 neighbor tests pass; both latency tests green in 3 of 3 rounds.
Follow-ups: none open from review.

@teamleaderleo teamleaderleo added the leo-named-fix An obvious fix to something Leo named: lands on push, review runs in parallel label Nov 4, 2026
…y session writers

Rebased onto feat-cmux-next 0a2b815; same tree as merge 93200c84.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo force-pushed the lane/tui-group-update-latency branch from 3e02bae to c9c6c68 Compare November 4, 2026 06:32
@cursor

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol xhigh, router-review): SHIP at c9c6c68

Range 0a2b815..c9c6c68 (the PR rebased onto base; same tree as the conflict resolution of 3e02bae). Checked lock handoffs, scope lifetimes and request paths; no findings.

Tests at this head: fast-checks run 41786996208, cargo test -p cmux-tui-core --lib -- group_update_latency_tests signaled_mutex: 6 passed, 0 failed.

Follow-ups: none

@teamleaderleo
teamleaderleo merged commit fb9c908 into feat-cmux-next Nov 4, 2026
198 of 200 checks passed
@teamleaderleo
teamleaderleo deleted the lane/tui-group-update-latency branch November 4, 2026 07:07
@lawrencecchen

Copy link
Copy Markdown
Contributor

The test this PR added, a_persons_requests_answer_while_busy_sessions_saturate_the_registry, is red in 9 of 31 full runs on feat (only the chat tab over its 200 ms p95). Two causes, neither in the priority lock: each chat tab create journaled every chat tab of its pane again (the public fold compared raw restatements against decorated values; #36212) and validated every tab of the pane (#36235). Both fixed; the test passes in 6 of 6 full runs with #36212.

(BASE RED SUITE for the cmux-next Performance Lead.)

This branch was previously deployed

3 inactive deployments
ci-routing-agent — c9c6c689 Deployed Nov 4, 2026 by github-actions[bot] via cmux-next macOS placement #69026
artifacts — c9c6c689 Deployed Nov 4, 2026 by teamleaderleo via publish tree-addressed cmux-tui #110127
sentry-dsyms — c9c6c689 Deployed Nov 4, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #110127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

leo-named-fix An obvious fix to something Leo named: lands on push, review runs in parallel risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants