cmux-tui: terminal host accept loop blocks on events (idle host woke 50x/s)

[lawrencecchen]

An idle terminal host woke about 50 times a second. The VM image lead measured it on a Freestyle VM baked from main (37ee6af9846): strace showed 488 poll(listener, 20 ms) timeouts and 489 accept4 = EAGAIN in 10 s on the warm template host's main thread.

Root cause: the host's accept loop polled its listener with a 20 ms timeout only to re-check dead and the launch-owner deadline, for the life of every terminal.

Fix (the terminal-host part of feat-cmux-next's 51b6863): the loop polls the listener plus an accept waker (a nonblocking socket pair that terminal exit and the last client stream's close write to). The launch-owner deadline is a one-shot timeout used only until it passes. The forced PTY drain also stops polling a hung-up drain waiter.

Test first (red then green): idle_template::parked_and_adopted_template_hosts_do_not_wake_while_idle parks a warm template host the way the bake does, counts its context switches over 10 s, then adopts it with a fresh-identity daemon and counts again. Each window allows 5 switches.

• Red on main + this test: 498 switches parked, 498 adopted (hosted run 37009612379).
• Green with the fix on feat-cmux-next: hosted run 37010846549 (Linux and macOS).

Depends on #16862 (main currently fails cargo fmt, which stops the hosted cmux-tui test jobs).

Changelog

Fixed: an idle terminal no longer wakes its terminal host process 50 times a second.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Stops an idle terminal from waking its terminal host process about 50 times a second by making the host's accept loop block on events instead of polling.

• The accept loop polled its listener with a 20 ms timeout only to re-check dead and the launch-owner deadline; it now blocks on the listener plus an accept waker.
• Terminal exit and the last client stream's close wake the loop through the waker, and the launch-owner deadline is now a one-shot timeout used only until it passes.
• The forced PTY drain also stops polling a hung-up drain waiter, which stayed readable and busy-looped the rest of the drain window.
• Adds a test that parks a warm template host, counts its context switches over 10 s, adopts it, then counts again, allowing 5 switches per window.

^{Written for commit 8fc5e56. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved terminal host recovery after the daemon restarts, helping adopted terminals return to a running state.
  • Reduced unnecessary background activity while terminal hosts are idle, including after a client closes its connection.
  • Prevented repeated wake-up events from disrupting terminal host shutdown and recovery.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (2)

docs/cloud-guest-upgrades.md — configured
.github/review-bot-rules/source-control-artifacts.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 08d95ae6-43e6-489f-9764-99c31a305a18

📥 Commits

Reviewing files that changed from the base of the PR and between b9ca453 and 8fc5e56.

📒 Files selected for processing (3)

• cmux-tui/crates/cmux-tui-core/src/terminal_host_runtime.rs
• cmux-tui/crates/cmux-tui/tests/terminal_host_recovery.rs
• cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/idle_template.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The runtime changes PTY forced-drain polling and replaces periodic host-listener polling with listener and lifecycle-waker polling. A recovery test measures context switches for parked and adopted template hosts on Linux and macOS.

Changes

Terminal host runtime

Layer / File(s)	Summary
Forced-drain waiter handling cmux-tui/crates/cmux-tui-core/src/terminal_host_runtime.rs	The drain poller stops polling a closed waiter descriptor during forced drain.
Accept-loop wakeups cmux-tui/crates/cmux-tui-core/src/terminal_host_runtime.rs	The runtime adds an AcceptWaker and signals it when the host exits or the active client-stream count reaches zero. The accept loop polls the listener and waker, waiting until the launch-owner deadline before resolution and indefinitely afterward.
Parked and adopted host measurement cmux-tui/crates/cmux-tui/tests/terminal_host_recovery.rs, cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/idle_template.rs	The recovery test measures context-switch counts over 10-second windows for parked and adopted hosts. It requires both counts to be at most five and waits for host-record cleanup after closing the surface.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant HostShared
  participant ActiveClientStream
  participant AcceptWaker
  participant HostAcceptLoop
  HostShared->>AcceptWaker: wake after host exit
  ActiveClientStream->>AcceptWaker: wake when active-stream count reaches zero
  AcceptWaker-->>HostAcceptLoop: reader descriptor becomes ready
  HostAcceptLoop->>AcceptWaker: drain pending bytes

Loading

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 8fc5e

The idle-host change is ready to merge after normal checks; no actionable risk remains identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8fc5e

The change is contained within terminal-host waiting and does not establish a new access path or weaken existing authentication. Remaining uncertainty concerns cleanup reliability under exceptional notification failures.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated change is scoped to each terminal host's local lifecycle and drain behavior. The new socket pair has no externally bound endpoint and does not establish additional service, tenant, or credential authority.

Trust Boundaries and Controls

• observed — The existing local listener uses private-directory preparation and mode 0600. Client authentication checks terminal identity and either the owner token or an accepted capability before command dispatch; sensitive terminal operations retain capability-right checks. These controls predate the waiting change.

Resilience and Maintainability Implications

• inferred — For successful or buffer-full notifications, pending bytes preserve wakeups across the state-check-to-poll interval, and draining precedes the next state evaluation. This supports cleanup under ordinary concurrent exit and final-stream closure without requiring one notification per transition. It does not establish recovery from every discarded write error.

Hardening Proposals

• proposed — Consider explicitly distinguishing a full notification buffer from interrupted or unexpected wake-write failures, with recovery behavior and focused fault-injection coverage for cleanup. This is additional assurance for indefinite waiting, not an established attacker-triggerable regression.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the terminal host accept-loop change and the idle wakeup problem.
Description check	✅ Passed	The description clearly explains the problem, root cause, implementation, regression test, test results, dependency, and changelog entry. It omits the template's explicit Demo Video and Checklist sect…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS. The diff only changes the terminal host's internal PTY drain polling and accept-loop wake mechanism, plus recovery tests. AcceptWaker is one lifecycle wake pair per host, not a new client, aut…
Cmux Swift Actor Isolation	✅ Passed	PASS: The authoritative PR diff changes only three Rust files (terminal_host_runtime.rs and two Rust test files). It introduces no Swift production changes, so it cannot introduce or worsen the spec…
Cmux Swift Blocking Runtime	✅ Passed	PASS: The authoritative PR diff changes only three Rust files (.rs) and contains no Swift paths or Swift runtime constructs. The Swift-specific blocking-runtime check is therefore not applicable.
Cmux Browser Automation Off-Main	✅ Passed	PASS: The authoritative PR diff changes only Rust terminal-host runtime and recovery-test files. It adds AcceptWaker, blocking poll behavior, PTY drain handling, and idle-host tests. It does not c…
Cmux Expensive Synchronous Load	✅ Passed	The authoritative PR diff changes only Rust files under cmux-tui; it contains no Swift files or production Swift changes. Therefore the Swift expensive synchronous load check is not applicable.
Cmux Cache Substitution Correctness	✅ Passed	PASS: The pull request changes only three Rust files (.rs). It introduces no production Swift, TypeScript, or JavaScript changes, so the cache-substitution correctness check is not applicable.
Cmux No Hacky Sleeps	✅ Passed	PASS: The authoritative PR diff changes only three Rust files (.rs). The custom check applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. It does not apply to these Rust r…
Cmux Algorithmic Complexity	✅ Passed	The production diff adds a fixed two-descriptor poll set for the listener and AcceptWaker, plus a bounded 64-byte wake drain. It does not add nested scans, repeated sorting/filtering, per-target r…
Cmux Swift Concurrency	✅ Passed	PASS: The review-scoped diff changes only Rust files under cmux-tui and adds no Swift code or Swift concurrency patterns. The Swift concurrency check is therefore not applicable.
Cmux Swift @Concurrent	✅ Passed	PASS: The reviewed diff changes only three Rust files and contains no Swift files. The Swift @concurrent check is not applicable.
Cmux Swift Package Boundaries	✅ Passed	The pull request changes only Rust production code and Rust tests under cmux-tui. It introduces no Swift or SwiftPM files, so the Swift package boundary rule does not apply.
Cmux Swiftpm Lockfiles	✅ Passed	The pull request changes only Rust runtime and Rust test files. The authoritative diff contains no Package.swift, Package.resolved, Xcode project/workspace, .gitignore, workflow, or dependency files. …
Cmux Swift Logging	✅ Passed	PASS: The review-scoped diff changes only three Rust files (.rs). It adds no Swift files or Swift logging statements, so the Swift logging rule does not apply.
Cmux User-Facing Error Privacy	✅ Passed	The changed production code only adds event-driven polling, wake descriptors, and lifecycle handling. It adds no user-facing error, alert, command output, API error body, or recovery copy. The new Fre…
Cmux Full Internationalization	✅ Passed	PASS. The authoritative diff changes only Rust terminal-host runtime behavior, internal comments/state, and a recovery test. It adds no Swift UI text, string-catalog or Info.plist entries, web UI/API …
Cmux Swiftui State Layout	✅ Passed	PASS: The pull request changes only three Rust files. The authoritative diff contains no Swift or SwiftUI paths and no SwiftUI state/layout constructs. The SwiftUI-specific failure conditions are not …
Cmux Architecture Rethink	✅ Passed	PASS. The authoritative PR diff changes only three Rust .rs files and contains no Swift, Objective-C, or SwiftUI/AppKit bridge code. The rule explicitly applies to Swift architecture changes, so its…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The authoritative pull-request diff changes only three Rust files under cmux-tui. It contains no Swift changes and adds or modifies no NSWindow, NSPanel, NSWindowController, SwiftUI Window, or W…
Cmux Source Artifacts	✅ Passed	All three changed paths are intentional Rust source or test files. The new file is a text-only recovery test under cmux-tui/crates/cmux-tui/tests/terminal_host_recovery/ and is wired into the existi…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The pull request changes only Rust files under cmux-tui/crates/.../src and tests. It changes no Swift file under a production **/Sources/** path, so this custom check is not applicable.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Merge receipt for 8fc5e560fc: every check was green at merge (13 verified; 20 skipped by policy). Full suite runs on main after merge.