Repository navigation
mux: Rust modernization — edition 2024, MSRV 1.88, latest deps, workspace lints - #7657
Conversation
… workspace lints Edition 2021->2024 across all crates incl. cmux-client (explicit unsafe blocks in FFI per unsafe_op_in_unsafe_fn). Deps: crossterm 0.29, ratatui 0.30.2, tungstenite 0.29 (Utf8Bytes/Bytes API), bindgen 0.72.1; libc stays 0.2 (1.0 is pre-release alpha). Workspace lints table (unsafe_op_in_unsafe_fn deny, uninlined_format_args, semicolon_if_nothing_returned, redundant_clone, needless_pass_by_value). cargo fmt. Wire/protocol/CLI unchanged; spec untouched.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe workspace is upgraded to Rust 2024 with shared lint and dependency settings. Several CDP and bindings paths now pass borrowed values instead of owned ones, and many mux-core, mux-tui, and ghostty-vt files were reformatted or had equivalent conditional guards rewritten. ChangesWorkspace and Manifest Upgrade
Borrowed-Reference API Refactor
Import Reordering and Conditional Formatting Sweep
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryThis PR modernizes the mux Rust workspace: edition 2021 → 2024, MSRV 1.88, and a dependency refresh (crossterm 0.29, ratatui 0.30, tungstenite 0.29, bindgen 0.72). Call-site churn from the tungstenite API change (
Confidence Score: 5/5Mechanical modernization with no protocol or behavioral changes; all API adaptations follow the tungstenite 0.29 migration path correctly and the unsafe env-mutation in tests is properly gated behind CONFIG_ENV_LOCK. Every tungstenite Message::Text send site converts String to Utf8Bytes via .into(); the Binary receive path is correct (only minor extra allocation). The let-chain refactors are semantically equivalent by construction. The unsafe_op_in_unsafe_fn lint is satisfied by existing unsafe {} blocks in the FFI wrappers, and the env::set_var wrapping in tests is backed by a real mutex. CI is the compile gate for the ghostty FFI crates as the author notes, but the Rust-side changes are sound. mux/crates/mux-cdp/src/client.rs — Binary message handler has a small unnecessary allocation (bytes.to_vec()); otherwise no files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant App as mux-tui App
participant CDP as CdpClient (mux-cdp)
participant WS as WebSocket (tungstenite 0.29)
participant Chrome as Chrome DevTools
App->>CDP: call(method, params)
CDP->>CDP: "json!({id, method, params:null})"
CDP->>WS: send(Message::Text(text.into()))
Note over WS: String to Utf8Bytes via .into()
WS->>Chrome: WebSocket frame
Chrome->>WS: WebSocket frame
alt Text frame
WS->>CDP: Message::Text(Utf8Bytes)
CDP->>CDP: handle_text via Deref to str
else Binary frame
WS->>CDP: Message::Binary(Bytes)
CDP->>CDP: from_utf8(bytes.to_vec()) then handle_text
end
CDP->>App: Result / CdpEvent
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant App as mux-tui App
participant CDP as CdpClient (mux-cdp)
participant WS as WebSocket (tungstenite 0.29)
participant Chrome as Chrome DevTools
App->>CDP: call(method, params)
CDP->>CDP: "json!({id, method, params:null})"
CDP->>WS: send(Message::Text(text.into()))
Note over WS: String to Utf8Bytes via .into()
WS->>Chrome: WebSocket frame
Chrome->>WS: WebSocket frame
alt Text frame
WS->>CDP: Message::Text(Utf8Bytes)
CDP->>CDP: handle_text via Deref to str
else Binary frame
WS->>CDP: Message::Binary(Bytes)
CDP->>CDP: from_utf8(bytes.to_vec()) then handle_text
end
CDP->>App: Result / CdpEvent
Reviews (4): Last reviewed commit: "mux-tui: collapse nested ifs into let-ch..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| edition.workspace = true | ||
| rust-version.workspace = true |
There was a problem hiding this comment.
cmux-client is the only crate in the workspace with publish = true, and it now inherits rust-version = "1.88" from the workspace. Any downstream consumer pinned to an older stable toolchain will see a hard rust-version rejection from Cargo 1.73+. This is intentional per the PR description, but the published CHANGELOG / crate docs should call it out explicitly so users aren't surprised on cargo update.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| let mut msg = json!({ | ||
| "id": id, | ||
| "method": method, | ||
| "params": params, | ||
| "params": null, | ||
| }); | ||
| msg["params"] = params; |
There was a problem hiding this comment.
The pattern of initialising "params": null and then overwriting with msg["params"] = params works correctly but is surprising to readers — null is a valid JSON params value and the extra step looks intentional. The actual motivation (suppressing clippy::needless_pass_by_value on the params: Value argument) is invisible at the call site. A brief comment explaining why the macro and direct assignment are split, or an #[allow(clippy::needless_pass_by_value)] on the function with a note, would make the intent clear without the two-step workaround.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…used_qualifications)
mux-core: collapse 13 nested ifs into let-chains (edition 2024 idiom), drop one redundant clone, remove needless_pass_by_value from workspace lints (signature churn). mux-tui: wrap test env set_var/remove_var in unsafe (unsafe fn in edition 2024; serialized by CONFIG_ENV_LOCK), inline format args in cli.rs print_tree, add missing statement semicolons in main.rs.
… (clippy round 3)
This branch was successfully deployed
1 participant
Modernization pass over the mux Rust workspace and cmux-client, no behavior change (spec untouched, conformance must stay green).
Local verification was limited to mux-cdp + cmux-client (this Mac's ghostty zig build is broken — known host libSystem regression), so CI is the compile gate for the FFI crates and the TUI dep churn.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Large dependency upgrades (especially ratatui 0.30 and tungstenite 0.29) and edition 2024 touch the TUI, CDP WebSocket path, and FFI build; risk is compile/runtime regressions in rendering and browser attach, not auth or data handling.
Overview
Modernizes the mux Rust workspace (cmux-client, ghostty-vt, mux-cdp/core/tui) without intended protocol or product behavior changes.
The workspace moves to Rust edition 2024 with
rust-version = 1.88, adds a shared[workspace.lints]table inherited by every crate, and bumps key deps: crossterm 0.29, ratatui 0.30 (split into core/crossterm/widgets crates), tungstenite 0.29, bindgen 0.72, plus a largeCargo.lockrefresh.Code changes are mostly edition/lint-driven:
let-chain style conditionals, import reordering,std::mem::size_of→size_of, explicitunsafein FFI whereunsafe_op_in_unsafe_fnis denied, and tungstenite API updates (Message::Textnow takes owned UTF-8; binary payloads use.to_vec()). CDP builds JSON-RPC params via indexed assignment instead of a singlejson!withparams.Chrome::launch_withtakes&ChromeLaunchOptionsinstead of owned options. Tests and config tests wrapset_var/remove_varinunsafeblocks per Rust 2024 rules.Reviewed by Cursor Bugbot for commit 8cd6b03. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit