cmux-tui: simplify JoinSet shutdown drains - #11745
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe preview proxy, mux bridge, and daemon shutdown paths now use ChangesTask shutdown paths
Merge Risk: ⚪ Minimal · up to Shutdown cleanup now uses Tokio’s built-in JoinSet shutdown helper at the existing abort-and-drain sites, preserving task termination and completion behavior while simplifying the implementation. No merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
Full details: Cmux Swift Actor IsolationExplanation PASS: The base-to-head diff changes only three Rust files, and all changed paths have the Full details: Cmux Swift Blocking RuntimeExplanation PASS. The declared diff changes only three Rust files: Full details: Cmux Browser Automation Off-MainExplanation PASS: The pull request changes only three Rust files under Full details: Cmux Expensive Synchronous LoadExplanation PASS: The pull-request diff changes only three Rust files under Full details: Cmux Cache Substitution CorrectnessExplanation PASS — The PR changes only three Rust files. The diff replaces Tokio Full details: Cmux No Hacky SleepsExplanation PASS: The pull request changes only Rust files. The diff replaces Full details: Cmux Algorithmic ComplexityExplanation PASS: The diff changes only three Rust ( Full details: Cmux Swift ConcurrencyExplanation PASS — The pull request diff contains only three Rust files and no Full details: Cmux Swift `@Concurrent`Explanation PASS: The pull request changes only three Rust files ( Full details: Cmux Swift Package BoundariesExplanation PASS: The pull request changes only three Rust files: Full details: Description checkExplanation The description explains what changed, identifies all four cleanup sites, preserves important exceptions, and documents verification results. It uses a "Verification" heading instead of the template's "Testing" heading and omits the review trigger and checklist, but the required change and testing information are mostly complete.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
a35248f to
a2bc5ef
Compare
This comment has been minimized.
This comment has been minimized.
2177a62 to
5c81d1f
Compare
5c81d1f to
90e856b
Compare
3cce67c Vault: recency-first All Sessions view, session search, and checkpoints with fork (manaflow-ai#10215) 94f51fb Fix aggregate child memory pressure before compressor exhaustion (manaflow-ai#10773) 13006ef cloud: surface whether after() has waitUntil for deferred create work (manaflow-ai#11782) 75eee0e Cloud VMs: bake the TigerVNC desktop (dock, wallpaper, cua-driver, noVNC) into the devbox recipe and open it at the machine's private address (manaflow-ai#11776) 36b5536 Fix terminal text bleed during live window resize (manaflow-ai#11530) 44b42c1 Cloud VMs: machines usage decoder and refresh fixes, edge smoke diagnostics (manaflow-ai#11759) f11be3a ci(tui): scope Valgrind test compilation (manaflow-ai#11750) 9184f4c coderouter: many Claude upstream accounts per team, routed with affinity and cooldown failover (manaflow-ai#11775) d3b9cdd cloud: attach waits for the baked supervisor; edge probe span joins the create trace (manaflow-ai#11777) c69e317 test: make the cmuxTests target compile again (main-actor call, CLI-only type) (manaflow-ai#11770) 8185825 fix(history): stop idle History menu graph rebuild loop (manaflow-ai#10661) 723958e Test bounded stale-port retirement after listener exit (manaflow-ai#11356) 367682e Fix native terminal Copy honoring Ghostty clipboard flavor (manaflow-ai#11515) d59055d docs(tui): refresh SDK inventory counts (manaflow-ai#11766) 89e4701 cmux-tui: use JoinSet shutdown for simple drains (manaflow-ai#11745) # Conflicts: # .github/workflows/cmux-tui.yml
Summary
Replace production
JoinSet::abort_all()plus ignoredjoin_next()drain loops withJoinSet::shutdown().awaitat the four equivalent cleanup sites:cmux-tui/crates/cmux-remote/src/bridge.rs:31-34,ForwardConnections::shutdowncmux-tui/crates/cmux-remote/src/bridge.rs:207,serve_mux_bridgecmux-tui/crates/cmux-remote/src/services.rs:332-334,DaemonServices::run_localcmux-tui/crates/chatmux-relay/src/preview_proxy.rs:477, preview proxy task cleanupError-observing joins and intentional graceful-then-forced two-phase shutdown paths remain explicit.
Tokio documents that
JoinSet::shutdownis equivalent to aborting all tasks and joining until the set is empty, while ignoring shutdown panics.Base:
b2a984ed59d6f7bfb109ebfbb90247dcc881465eHead:
90e856ba3973d56e11a2264a11f1d4e5e839b0f2Verification
rustfmt --edition 2024 --checkon all three changed Rust files: passedgit diff --check: passedcargo fmt --all -- --check: blocked by the local cmux-tui artifact guard because free space was 159.3 GiB, below the 250 GiB floor