Repository navigation
Iroh: full-stack coverage, a client that builds without Zig, and an iOS app - #9237
lawrencecchen wants to merge 13 commits into
Conversation
Every other provider has an end-to-end test that drives the real daemon through invitation enrollment, Noise, and the service layer. Iroh had none: its inline tests cover carrier mechanics but all bind with RelayMode::Disabled on loopback and stop at raw frames, so a relayed path and a PTY driven across one were never exercised. Both tests run against a relay server spawned in-process over plaintext HTTP, so they need no network and no certificate handling.
cmux-remote pulled in libghostty-vt unconditionally, but the only code that uses it is the workspace service's server-side terminal model. A client dialing a daemon needs none of that, and on iOS the dependency is not merely dead weight: it makes a Rust toolchain and the iOS SDK insufficient to build a client, because libghostty-vt is a Zig build. The new default-on daemon-services feature carries the workspace service, the handlers built on it, and the local bridge. MessageStream and ServicesError move to a new message module, since both endpoints need them and only their old home was daemon-side. Building without the feature leaves the carrier, the authenticated session, and WorkspaceClient intact, and needs no Zig at all.
The daemon protocol is not something a phone should reimplement: enrollment is a PSK-authenticated Noise handshake, sessions are mutually authenticated and resumable, frames carry per-lane sequence numbers with bounded replay, and Iroh adds path selection and relay fallback under all of it. A second implementation in Swift would be a second set of bugs in the parts hardest to test. The phone links the same Rust client the TUI uses, and this crate is only the boundary: connect, attach a shell, move bytes, resize, and read a connection snapshot. Verified by building the crate for aarch64-apple-ios, which produces an arm64 archive exporting all nine entry points. ghostty-vt-sys also gains CMUX_GHOSTTY_VT_LIB_DIR, which links a prebuilt archive instead of invoking zig. zig 0.15.2 cannot link a host executable on macOS 26, and `zig build` builds its own runner for the host before anything else, so a Mac on 26 cannot run it even to cross-compile.
The transport's interesting failures happen on the network a phone has: NAT'd, moving between cellular and Wi-Fi, often unable to reach the daemon directly at all. That is what relay fallback and session resume are for, and none of it occurs on a loopback test. The app implements no part of the daemon protocol. It links cmux-remote-mobile, so enrollment, Noise, reliability, and path selection are the same code the TUI runs. It carries no VT parser either: the daemon answers SnapshotProcessTerminal with styled runs, so the phone lays out a monospaced grid and nothing more. The status bar names the selected path and counts session resumes, and the path picker forces direct-only or relay-only, so a failure can be attributed to one path rather than to "the network". Verified by type-checking every source under Swift 6 for arm64-apple-ios. A full Xcode build is blocked locally by the repository's xcodebuild guard, so the app bundle itself is unbuilt here.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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.
|
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 41058ee. Configure here.
| LIBRARY_SEARCH_PATHS: >- | ||
| $(SRCROOT)/../../target/$(CMUX_RUST_TARGET)/$(CMUX_RUST_PROFILE) | ||
| OTHER_LDFLAGS: -lcmux_remote_mobile | ||
| CMUX_RUST_PROFILE: debug |
There was a problem hiding this comment.
Release links wrong Rust archive
High Severity
CMUX_RUST_PROFILE is hardcoded to debug while build-rust.sh passes --release for non-Debug Xcode configurations. Release builds search target/.../debug for libcmux_remote_mobile.a after cargo wrote the archive under release, so linking fails or picks a stale debug library.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 41058ee. Configure here.
| } | ||
| connection = await client.connection() | ||
| } | ||
| } |
There was a problem hiding this comment.
Idle pump still polls RPCs
Medium Severity
The pump comment claims an idle shell costs nothing, but after every readOutput timeout it still awaits terminal() and connection(), each of which crosses into Rust and hits the daemon. On a phone that becomes a SnapshotProcessTerminal plus diagnostics RPC about every 250ms while the shell is idle.
Reviewed by Cursor Bugbot for commit 41058ee. Configure here.
| text.foregroundColor((run.fg ?? snapshot.defaultFg).swiftUI), | ||
| at: CGPoint(x: x, y: y), | ||
| anchor: .topLeading) | ||
| column += run.text.count |
There was a problem hiding this comment.
Wide cells use character count
Medium Severity
Column advancement uses run.text.count instead of the daemon-provided width_hint on each styled run. Wide glyphs and combining sequences then misalign the grid relative to the server-side terminal model, so cursor and background fills drift for non-narrow text.
Reviewed by Cursor Bugbot for commit 41058ee. Configure here.
* fix(tui): normalize workspace mode constants * fix(tui): specify normalized mode type --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Test SSH bootstrap cancellation at child owner * Fix Future bounds under pinned Clippy
* test: cover deferred provider selection readiness * fix: retain provider selection until ready * test: hold ready provider through observation * test: cover explicit provider switch override * fix: cancel deferred selection after switch * test: prove provider reconnect is one-shot * test: retain unready provider selection intent * fix: commit provider selection cancellation
* test: await published browser tab state * test: use the close-tab owner action
|
Closing this stale stacked implementation. Head |
|
Fleet instruction update for head |
3 participants
Stacked on #8667. Targets
feat-cmux-tui-remote-daemon, notmain.The goal was to get the Iroh transport onto a phone. Three things were in the way, and each is a commit.
Iroh had no end-to-end test
direct_ws_e2e,direct_wss_e2e,relay_e2e, andpty_reconnect_e2eeach drive the real daemon through invitation enrollment, Noise, and the service layer. Iroh had none. Its inline tests cover carrier mechanics well, but every one of them binds withRelayMode::Disabledon loopback and stops at raw frames, so the two things a phone depends on were never exercised: a relayed path, and a PTY driven across one.crates/cmux-remote/tests/iroh_e2e.rsadds both, against a relay server spawned in-process over plaintext HTTP so the tests need no network and no certificate handling.A client could not build without a Zig toolchain
cmux-remotedepended onghostty-vtunconditionally, but the only code that touches it is the workspace service's server-side terminal model. On iOS that is not merely dead weight: libghostty-vt is a Zig build, so a Rust toolchain and the iOS SDK were not enough to build a client.The new default-on
daemon-servicesfeature carries the workspace service, the handlers built on it, and the local bridge.MessageStreamandServicesErrormove to a newmessagemodule, since both endpoints need them and only their old home was daemon-side. Without the feature the carrier, the authenticated session, andWorkspaceClientare intact and no Zig is involved.ghostty-vt-sysalso gainsCMUX_GHOSTTY_VT_LIB_DIR, which links a prebuilt archive instead of invoking zig, mirroring how the Swift side consumes a prebuilt GhosttyKit. Worth knowing: zig 0.15.2 cannot link a host executable on macOS 26, andzig buildbuilds its own runner for the host before anything else, so a Mac on 26 cannot run it even to cross-compile.The phone should not reimplement the protocol
Enrollment is a PSK-authenticated Noise handshake, sessions are mutually authenticated and resumable, frames carry per-lane sequence numbers with bounded replay, and Iroh adds path selection and relay fallback underneath. A Swift reimplementation would be a second set of bugs in the parts hardest to test.
crates/cmux-remote-mobileis a C ABI over the same Rust client the TUI uses: connect, attach a shell, move bytes, resize, and read a snapshot.apps/iosis a SwiftUI app on top of it. It carries no VT parser either, because the daemon answersSnapshotProcessTerminalwith styled runs, so the app lays out a monospaced grid and nothing more.Its status bar names the selected path and counts session resumes, and the path picker forces direct-only or relay-only, so a failure can be attributed to one path instead of to "the network".
Verification
cmux-remotecompiles with--no-default-features --features iroh-transportwith zig removed fromPATHentirely.cmux-remote-mobilebuilds foraarch64-apple-ios, producing an arm64 archive that exports all ten entry points.arm64-apple-ios.Rust verification ran in a Linux container, because this Mac is on macOS 26 where zig cannot link at all. That is the same reason the iOS app bundle is unbuilt here:
xcodebuildis blocked locally by the repository guard, so the app has been compiled but not run on a device. That is the one gap.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Brings the Iroh transport to iOS with a Zig‑free client build, full‑stack tests, a new
cmux-remote-mobileC ABI, and a SwiftUI app. Also defers provider selection until connectable, normalizes workspace mode constants, makes initial route timeouts owner‑driven, adds cancellable SSH bootstrap tests, updates Cloudflare relay tooling towrangler4.120.0, and stabilizes the TUI smoke test by waiting for published browser tab state.New Features
cmux-remote-mobile: a static C ABI over the Rust client (connect, attach shell, IO, resize, snapshots). Builds foraarch64-apple-iosand simulator.apps/ios: uses the C ABI, shows selected path and resume count, lets you force auto/direct‑only/relay‑only, and renders the terminal from styled runs.crates/cmux-remote/tests/iroh_e2e.rs) covering direct and relay paths with a real PTY via an in‑process relay.EnrollmentInvitation::daemon_public_key_bytesto support pinning the daemon key at connect.SshBootstrapCancellation) with a test that shuts it down at its child owner.interactive_latency_e2eto track RAM under load.Migration
daemon-servicesfeature gates host‑only services and Zig. Client‑only builds:cargo build -p cmux-remote --no-default-features --features iroh-transport.libghostty-vt.ainstead of invoking Zig, setCMUX_GHOSTTY_VT_LIB_DIRinghostty-vt-sys’s build environment.ghostty-vt-sysbuild now accepts borrowed paths in its Zig build configuration.Written for commit d3b8c2c. Summary will update on new commits.
Note
Medium Risk
Touches authenticated remote session/Iroh transport and splits
cmux-remotefeatures; default daemon builds stay the same, but client-only and iOS linking paths need CI coverage. iOS app is largely new surface (network, enrollment, terminal I/O) and was not run on device in the author’s environment.Overview
Adds an iOS cmux remote app that enrolls via
cmux://enroll/…, opens a shell over Iroh, and renders the terminal from daemon styled-run snapshots (no on-device VT parser). Swift talks tocmux-remote-mobile, a static Rust library with a small C ABI over the same client stack as the TUI.cmux-remotenow gates host-only workspace/VT work behinddaemon-services(default on) soiroh-transportalone can build a mobile client without pulling ghostty-vt / Zig. SharedMessageStream/ServicesErrormove tomessage.ghostty-vt-syscan link a prebuilt archive viaCMUX_GHOSTTY_VT_LIB_DIR.EnrollmentInvitation::daemon_public_key_bytessupports pinning at connect.New
iroh_e2etests drive a real daemon PTY over Iroh on direct and relay paths (in-process plaintext relay), matching coverage other transports already had.Reviewed by Cursor Bugbot for commit a7e63ee. Bugbot is set up for automated code reviews on this repo. Configure here.