Native Swift frontend for cmux TUI - #9785
Closed
lawrencecchen wants to merge 221 commits into
Closed
lawrencecchen wants to merge 221 commits into
lawrencecchen wants to merge 221 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a shared terminal-host protocol crate, native frontend and terminal C APIs, a macOS SwiftUI NativeMuxDemo, local and remote launch workflows, lifecycle verification, and viewport-width resource support. ChangesNative frontend and terminal protocol
NativeMuxDemo application
Demo workflows
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant runRemoteDemo as run-remote-demo.sh
participant remoteOwner as remote-daemon-owner.sh
participant daemon as cmux daemon
participant nativeDemo as NativeMuxDemo
participant frontendClient as cmux-terminal-client
runRemoteDemo->>remoteOwner: Launch daemon with ownership channel
remoteOwner->>daemon: Start and monitor daemon
runRemoteDemo->>daemon: Create workspace and invitation
runRemoteDemo->>nativeDemo: Launch with invitation and autoconnect
nativeDemo->>frontendClient: Connect and request resources
frontendClient->>daemon: Send lane-aware JSON requests
daemon-->>frontendClient: Return snapshots and render events
frontendClient-->>nativeDemo: Deliver terminal updates
nativeDemo->>remoteOwner: Disconnect on app shutdown
remoteOwner->>daemon: Shut down daemon and terminal hosts
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (10 errors, 2 warnings)
✅ Passed checks (13 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 41
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux-tui/apps/macos/NativeMuxDemo/Package.swift`:
- Around line 17-23: The xcframework paths are inconsistent and the Rust linker
path hardcodes the debug profile. In Package.swift, replace the relative
unsafeFlags paths with paths derived from a documented, verified build root and
select the Rust artifact without hardcoding debug. In
Sources/GhosttyKit/module.modulemap at line 2, verify and adjust the six-segment
xcframework path so it resolves to the same GhosttyKit.xcframework used by
Package.swift.
In `@cmux-tui/apps/macos/NativeMuxDemo/README.md`:
- Around line 1-86: The README currently provides only English operational
documentation. Add a Japanese locale-specific document matching the English
README’s launch, lifecycle, remote-demo, cleanup, commands, and safety
constraints, then update the package entry point to link both locale documents.
Follow the repository’s existing locale-specific Markdown naming and linking
conventions, and update every supported locale entry required by those
conventions.
In `@cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh`:
- Around line 364-383: Update the retry loops in run-demo.sh lines 364-383 and
run-remote-demo.sh lines 431-432 and 447-452: append || true to each enroll
pending/status command substitution so transient admin-socket or SSH failures
consume the retry budget instead of aborting under set -e. In both enroll status
loops, validate the captured value with a numeric regex before evaluating the
CONNECTED arithmetic comparison; no direct change is required at run-demo.sh
lines 364-383 beyond its pending substitution guard.
In `@cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh`:
- Around line 256-273: Update the cleanup function’s final status handling so
its exit trap explicitly terminates with exit_status after performing all
cleanup and log reporting. Replace the current return-based completion in
cleanup with an explicit exit, preserving the existing exit_status values set by
daemon-alive and remote-removal failures.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swift`:
- Around line 48-49: Update the URL construction in BrowserView so the remote
browser.url is loaded only when its scheme is http or https; reject all other
schemes, including file and javascript, before creating NativeWebView.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/DemoWindowPlacement.swift`:
- Around line 11-28: The window-placement flow in applyIfConfigured() must
select the target window before choosing its screen, derive visibleFrame from
window.screen rather than NSScreen.screens.first, and pass that frame to
fit(visibleFrame:). Update fit(visibleFrame:) so ghosttyPositionX includes
visibleFrame.minX plus nativeWidth and gap, and preserve this behavior with a
non-zero-origin test.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendModel.swift`:
- Around line 226-242: Update selectWorkspace and selectScreen to track the
prior selection and explicitly restore it when their focus mutation fails, using
the mutation’s failure reporting through errorMessage or the existing mutation
completion path. Ensure workspace and screen selections are rolled back together
as needed, while preserving the optimistic selection on successful mutations.
- Around line 447-458: In the post-mutation success path of mutate, replace the
direct refreshNow() call with scheduleRefresh(). Preserve the existing mutation
request and error handling so refreshes are coalesced and serialized by
scheduleRefresh.
- Around line 465-483: Update the application termination flow around
FrontendModel.shutdown and the app delegate’s termination callbacks so closing
the last window uses applicationShouldTerminate with .terminateLater, then
signals termination only after the shutdown task has awaited all controller
shutdowns and ownedService.shutdown(). Preserve the existing shutdown guard and
cleanup behavior while ensuring windowWillClose cannot allow process termination
before teardown completes.
- Around line 85-131: Store the connection Task created in the connect flow as a
lifecycle property, and cancel it from shutdown() alongside updatesTask and
refreshTask. Ensure the task clears or remains safely handled after completion,
and preserve cancellation/error handling so it cannot install callbacks or
update transportDiagnostics/isConnecting after shutdown.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendService.swift`:
- Around line 100-160: Move the blocking FFI calls in request, attachTerminal,
and TerminalHandle send/resize methods off the actor’s cooperative thread pool
using the existing connect Task.detached pattern or a dedicated executor.
Preserve their current parameters, timeout, error decoding, and return behavior
while ensuring each network operation has an explicit executor boundary.
- Line 129: Update the response-payload conversion in the session
snapshot/mutation handling to construct Data directly from the result pointer,
removing the intermediate String(cString:) and utf8 conversion. Preserve the
existing null-terminated C-string semantics and resulting JSON payload behavior.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRemoteSurfaceView.swift`:
- Around line 507-522: Update the mouseButton mapping so button numbers 3
through 8 map sequentially to GHOSTTY_MOUSE_FOUR through GHOSTTY_MOUSE_NINE,
while preserving the existing mappings for 0–2, 9–10, and the default unknown
case.
- Around line 184-186: Guard the optional surface handle before calling
libghostty APIs in the .ready handler, viewDidMoveToWindow(),
becomeFirstResponder(), and resignFirstResponder(). Only invoke refresh, focus,
or other surface operations when surface is non-nil; otherwise leave the
runtime/surface state unchanged and avoid passing nil handles.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRuntime.swift`:
- Around line 21-23: Add a concise comment beside the NO_COLOR getenv/unsetenv
block explaining why it must be removed before ghostty_init and acknowledging
that this modifies the process environment, or replace the global environment
mutation with a libghostty-specific configuration that disables NO_COLOR without
affecting user preferences or child processes.
- Around line 46-61: Cancel the already-created ticker on every initializer
failure path before returning nil. In the fallback branch around
loadFallbackConfiguration() and fallback ghostty_app_new, call ticker.cancel()
before each failure return, while preserving the existing configuration cleanup
and successful NativeGhosttyRuntimeLifetime assignment.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/LayoutViews.swift`:
- Around line 38-114: Replace the top-level GeometryReader in the viewport
layout around the ForEach columns with localized geometry measurement, using
onGeometryChange or a contained background measurement to obtain the container
size without owning the entire layout; preserve naturalWidth, renderedWidth, and
height calculations. Apply the same localized measurement pattern to the split
container at LayoutViews.swift lines 182-198, with no other layout behavior
changes.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeMuxDemoApp.swift`:
- Around line 43-47: Replace the deferred Task in the window creation flow with
lifecycle-based placement tied to the created NSWindow, using a dedicated window
lifecycle callback or by passing that NSWindow directly into
DemoWindowPlacement.applyIfConfigured(). Ensure placement cannot run after the
window is closed and does not resolve a stale keyWindow or windows.first.
- Around line 26-36: Assign the standalone NSWindow in NativeMuxDemoApp to a
stable cmux.* identifier and add the identical identifier to
cmuxAuxiliaryWindowIdentifiers so cmuxWindowShouldOwnCloseShortcut(_:)
recognizes it as auxiliary. Do not add a custom Cmd+W handler.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swift`:
- Around line 59-81: Store the attachment lifecycle Task created by attach() in
a dedicated property, retaining its existing attachTerminal and consumeUpdates
flow. Update shutdownAndWait() to cancel that stored task alongside updateTask
and inputTask, and clear the reference when the task completes or shutdown
finishes.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/en.lproj/Localizable.strings`:
- Line 30: Define explicit .one and .other pluralization entries for
spaces.count and columns.count in the English catalog, and add matching entries
in the Japanese catalog; update every count lookup call site to select the
appropriate key based on the count. Apply these changes in
en.lproj/Localizable.strings at lines 30-30 and 35-35, and
ja.lproj/Localizable.strings at lines 30-30 and 35-35.
- Line 26: Remove Iroh, libghostty, and Ghostty implementation names from
user-visible localization strings. Update the ready status at
cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/en.lproj/Localizable.strings:26-26
and the error copy at :43-44 to use product-level terms; apply matching ready
and error translations at
cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/ja.lproj/Localizable.strings:26-26
and :43-44, while retaining provider details only in sanitized diagnostics.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/ResourceSnapshot.swift`:
- Around line 104-122: Update FrontendModel snapshot ingestion to build and
retain precomputed indexes named screensByWorkspaceID, tabsByPaneID, panesByID,
terminalsByID, and browsersByID, sorting grouped screen and tab arrays by index
once. Refactor screens(in:), tabs(in:), pane(_:), terminal(for:), and
browser(for:) in ResourceSnapshot to read from these indexes instead of
filtering, sorting, or scanning the full collections on each call.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/SpacesBar.swift`:
- Around line 68-70: Update the Label localization in SpacesBar.swift (lines
68-70) to select columns.count.one when columns.count is one and
columns.count.other otherwise, preserving the count argument. Apply the same
explicit plural-key selection in WorkspaceSidebar.swift (lines 62-66), using
spaces.count.one for one space and spaces.count.other for all other counts.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/TerminalView.swift`:
- Around line 41-47: Update TerminalView’s rendering of terminal.errorMessage
and the NativeTerminalModel.attach() error flow so user-facing text uses
localized product-safe messages rather than raw error.localizedDescription.
Preserve sanitized diagnostic details only in internal logs, and ensure no
vendor, transport, or upstream error details reach the displayed Text.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/WorkspaceSidebar.swift`:
- Around line 26-28: Remove the per-render sorted call from the ForEach in
WorkspaceSidebar and iterate snapshot.workspaces directly. Ensure workspaces are
ordered by index when the snapshot is decoded or refreshed, preserving the same
ordering without repeated sorting in body updates.
- Around line 62-66: Update the workspace sidebar count text in the relevant
view to select the explicit spaces.count.one localization key when the screen
count is 1 and spaces.count.other otherwise, preserving the current count
interpolation. Add both keys with appropriate singular and plural translations
to every supported locale catalog.
- Around line 26-27: The workspace rows currently capture FrontendModel inside
workspaceButton(_:) under the LazyVStack. Replace that row implementation with a
value-only view that receives the selected workspace, screen count, and
select/close action closures as parameters, and update the LazyVStack call site
to pass immutable snapshot values and those closures instead of the model
reference.
In `@cmux-tui/apps/macos/NativeMuxDemo/test-remote-command.sh`:
- Around line 40-55: Remove the dead fallback branch after the `RUN_REMOTE_DEMO`
source path, including the `eval`, temporary environment variables, and
`remote_under_test` wrapper. Keep only the sourced `remote-command.sh` helper
path, and remove the now-unused `RUN_REMOTE_DEMO` variable declaration.
In `@cmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.sh`:
- Around line 10-24: Move the argument validation block before the
TEST_ROOT="$(mktemp -d ...)" initialization so invalid invocations exit before
creating temporary resources. Keep the existing validation behavior and usage
message unchanged, and leave the remaining lifecycle setup intact.
In `@cmux-tui/crates/cmux-remote/Cargo.toml`:
- Around line 14-15: Guard every use of the optional crates cmux_pty,
cmux_tui_core, and ghostty_vt with the daemon-services feature, keeping their
existing behavior when that feature is enabled. Add a CI check that builds
cmux-remote with --no-default-features so unguarded references are detected.
In `@cmux-tui/crates/cmux-terminal-client/Cargo.toml`:
- Around line 13-29: Gate the text-renderer implementation consistently on the
text-renderer feature: update lib.rs imports, key_input_from_chord, ClientState
fields and constructors, and the frontend module so they are only compiled when
enabled. Ensure native-renderer builds without ghostty_vt or text-renderer-only
symbols, while no-default-features builds also resolve cleanly.
In `@cmux-tui/crates/cmux-terminal-client/src/frontend.rs`:
- Around line 780-785: Update cmux_frontend_terminal_has_exited to return the
authoritative ClientState.exited boolean instead of comparing the diagnostic
status string, while preserving the existing null-pointer fallback of false.
- Around line 268-286: Update the request flow around the pending insertion and
control stream lookup so an unavailable mux-control cannot leave the new id in
control_state.pending. Resolve the stream before inserting the pending sender,
or explicitly remove the id when the lookup returns an error; preserve the
existing cleanup for send failures.
- Around line 369-375: Update the nonce-generation error branch in the
surrounding frontend connection setup to perform the same transport cleanup as
the control-stream failure path: shut down the multiplexer, connection,
provider, and Tokio runtime, and unregister the daemon stream before returning
null. Preserve copying the error into error_buffer via copy_utf8.
In `@cmux-tui/crates/cmux-terminal-client/src/lib.rs`:
- Around line 513-525: Update the MessageKind::Exit branch in apply so it always
returns FrameEffect::Stop after marking the terminal exited, regardless of the
result from continue_after_native_event. Do not propagate Restart or other
queue-overflow effects for this terminal event; reserve resynchronization for
non-terminal frames.
- Around line 424-429: Update the Reset event construction in
continue_after_native_event to include the complete Kitty rendering state from
snapshot, including kitty_image_aliases and kitty_state alongside
snapshot.replay. Ensure native renderers reconstruct the same image aliases and
replay state used by snapshot.apply_vt_replay_parts().
In `@cmux-tui/crates/cmux-terminal-host-protocol/Cargo.toml`:
- Around line 13-19: Move serde_json from [dependencies] to [dev-dependencies]
in the crate manifest, keeping it available for the #[cfg(test)] usage while
removing it from runtime dependencies.
In `@cmux-tui/crates/cmux-terminal-host-protocol/src/lib.rs`:
- Around line 703-724: Refactor FrameDecoder’s header state and decode flow to
retain the Header parsed during initial validation, eliminating the second
parse_header call and the drain-based payload memmove. Keep header bytes
separate from payload storage, while preserving early rejection of oversized
lengths and ensuring buffered_len() remains HEADER_LEN when rejection occurs, as
covered by oversized_length_is_rejected_before_payload_is_buffered.
- Around line 206-212: Update encode_terminal_exit’s
TerminalExitOutcome::Unknown branch to ensure the encoded reason is non-empty
before appending it, using the existing default reason established by
TerminalExit::unknown. Preserve UTF-8 truncation and ensure the resulting
payload always exceeds EXIT_PAYLOAD_HEADER_LEN so decode_terminal_exit accepts
it.
- Around line 15-28: Move the terminal-host handshake types CapabilityRights,
CapabilityToken, ClientHello, ClientRole, HostHello, HostIncarnation, and
TerminalId from cmux_tui_core::terminal_host into the
cmux-terminal-host-protocol crate alongside the existing protocol constants and
payload definitions. Update their visibility and all imports/re-exports so
cmux-remote and other callers use the protocol crate as the single source for
the complete handshake.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 91939b2b-7c54-4698-9fea-8026da75b859
⛔ Files ignored due to path filters (1)
cmux-tui/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
cmux-tui/Cargo.tomlcmux-tui/apps/macos/NativeMuxDemo/Package.swiftcmux-tui/apps/macos/NativeMuxDemo/README.mdcmux-tui/apps/macos/NativeMuxDemo/Sources/CCmuxTerminal/cmux_terminal_client.hcmux-tui/apps/macos/NativeMuxDemo/Sources/CCmuxTerminal/module.modulemapcmux-tui/apps/macos/NativeMuxDemo/Sources/GhosttyKit/module.modulemapcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/DemoWindowPlacement.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendService.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRemoteSurfaceView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRuntime.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/JSONValue.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/LayoutNode.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/LayoutViews.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Localization.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeMuxDemoApp.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/PaneView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/ResourceSnapshot.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/en.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/ja.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/RootView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/SpacesBar.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/TerminalView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/WorkspaceSidebar.swiftcmux-tui/apps/macos/NativeMuxDemo/Support/Info.plistcmux-tui/apps/macos/NativeMuxDemo/Support/en.lproj/InfoPlist.stringscmux-tui/apps/macos/NativeMuxDemo/Support/ja.lproj/InfoPlist.stringscmux-tui/apps/macos/NativeMuxDemo/Tests/NativeMuxDemoTests/NativeMuxDemoTests.swiftcmux-tui/apps/macos/NativeMuxDemo/launch-ghostty-client.shcmux-tui/apps/macos/NativeMuxDemo/remote-command.shcmux-tui/apps/macos/NativeMuxDemo/remote-daemon-owner.shcmux-tui/apps/macos/NativeMuxDemo/remote-lifecycle-ready.shcmux-tui/apps/macos/NativeMuxDemo/run-demo.shcmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.shcmux-tui/apps/macos/NativeMuxDemo/test-remote-command.shcmux-tui/apps/macos/NativeMuxDemo/test-remote-lifecycle-ready.shcmux-tui/apps/macos/NativeMuxDemo/verify-demo-lifecycle.shcmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.shcmux-tui/bindings/cpp/.cmux-resource-api.jsoncmux-tui/bindings/go/.cmux-resource-api.jsoncmux-tui/bindings/java/.cmux-resource-api.jsoncmux-tui/bindings/python/.cmux-resource-api.jsoncmux-tui/bindings/rust/.cmux-resource-api.jsoncmux-tui/bindings/typescript/.cmux-resource-api.jsoncmux-tui/bindings/zig/.cmux-resource-api.jsoncmux-tui/crates/cmux-remote/Cargo.tomlcmux-tui/crates/cmux-remote/src/lib.rscmux-tui/crates/cmux-remote/src/mux_codec.rscmux-tui/crates/cmux-remote/src/service.rscmux-tui/crates/cmux-terminal-client/Cargo.tomlcmux-tui/crates/cmux-terminal-client/include/cmux_terminal_client.hcmux-tui/crates/cmux-terminal-client/src/frontend.rscmux-tui/crates/cmux-terminal-client/src/lib.rscmux-tui/crates/cmux-terminal-host-protocol/Cargo.tomlcmux-tui/crates/cmux-terminal-host-protocol/src/lib.rscmux-tui/crates/cmux-tui-core/Cargo.tomlcmux-tui/crates/cmux-tui-core/src/lib.rscmux-tui/crates/cmux-tui-core/src/resource_router.rscmux-tui/crates/cmux-tui-core/src/surface.rscmux-tui/crates/cmux-tui-core/src/terminal_host_protocol.rscmux-tui/scripts/check-spec-inventory.pycmux-tui/scripts/test_check_spec_inventory.pycmux-tui/spec/resource-operations-v2.jsoncmux-tui/spec/terminal-host.md
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (5)
cmux-tui/apps/macos/NativeMuxDemo/test-remote-command.sh (1)
40-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dead
evalfallback branch.This PR adds
remote-command.shin the same directory, so line 29 always succeeds and lines 40-55 never run. The branch also re-parses launcher text througheval, which static analysis flags. Delete the branch and keep only the sourced helper path.RUN_REMOTE_DEMOat line 6 then becomes unused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/apps/macos/NativeMuxDemo/test-remote-command.sh` around lines 40 - 55, Remove the unreachable fallback branch beginning at the else clause, including its sed/eval extraction, test environment setup, and remote_under_test wrapper. Keep the sourced remote-command.sh path as the sole setup in the test script, and remove the now-unused RUN_REMOTE_DEMO variable.Source: Linters/SAST tools
cmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.sh (1)
10-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe usage-error path leaks the temporary directory.
Line 10 creates
TEST_ROOT. Lines 21-24 canexit 2before line 45 installs theEXITtrap. The directory then stays inTMPDIR. Validate the arguments before you createTEST_ROOT.🧹 Proposed fix
RUNS="${2:-3}" +if [[ $# -gt 2 || ! "$RUNS" =~ ^[1-9][0-9]*$ ]]; then + echo "Usage: verify-remote-demo-lifecycle.sh [ssh-host] [runs]" >&2 + exit 2 +fi TEST_ROOT="$(mktemp -d "${TMPDIR:-/tmp}/cmux-native-remote-lifecycle.XXXXXX")" @@ READY_POLL_SECONDS=0.1 - -if [[ $# -gt 2 || ! "$RUNS" =~ ^[1-9][0-9]*$ ]]; then - echo "Usage: verify-remote-demo-lifecycle.sh [ssh-host] [runs]" >&2 - exit 2 -fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.sh` around lines 10 - 24, Move the argument validation in verify-remote-demo-lifecycle.sh before the TEST_ROOT assignment and any temporary-directory setup. Preserve the existing usage message and exit status, while ensuring invalid arguments cannot create an uncleaned temporary directory.cmux-tui/apps/macos/NativeMuxDemo/README.md (1)
1-86: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdd matching Japanese operational documentation.
This README documents launch, lifecycle, and remote cleanup only in English. The app ships
en.lprojandja.lprojcatalogs. Add a Japanese document with the same commands and safety constraints, and link both documents from the package entry point.As per coding guidelines, "User-facing text must use localized APIs and matching catalogs; web, metadata, API, markdown, changelog, and user-facing data must use locale-specific sources and update every supported locale."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/apps/macos/NativeMuxDemo/README.md` around lines 1 - 86, Add a Japanese README alongside the existing NativeMuxDemo documentation that mirrors all launch, lifecycle, remote-demo, cleanup, command, and safety details in README.md, using the same commands without alteration. Update the package entry point to link both the English and Japanese documents, and ensure the Japanese document is included through the app’s existing ja.lproj/catalog structure where applicable.Source: Coding guidelines
cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh (1)
256-273: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
returnin the EXIT trap does not change the script exit status.
cleanupruns only as theEXITtrap. The shell exit status stays the status that triggered the trap.returndoes not override it. Lines 260 and 262 setexit_status=1when the remote daemon is still alive or when the remote state removal fails, but the script still exits 0.verify-remote-demo-lifecycle.shline 193 checkswait "$LAUNCHER_PID", so it reports a clean run after a failed remote cleanup.🐛 Proposed fix
if [[ "$LOCAL_ROOT" == "$TEMP_PARENT"/cmux-native-remote-client.* ]]; then rm -rf -- "$LOCAL_ROOT" fi - return "$exit_status" + exit "$exit_status" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh` around lines 256 - 273, Update the EXIT-trap cleanup function around cleanup so failures recorded in exit_status override the final process status, rather than relying on return. Preserve the existing daemon-alive and remote-removal checks, but explicitly terminate or propagate exit_status from the trap when it is nonzero so verify-remote-demo lifecycle checks observe the cleanup failure.cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh (1)
364-383: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRetry loops abort on the first transient failure. Each poll loop assigns the result of a command substitution while
set -eis active. One failed admin-socket call or one failed SSH round trip ends the script, so the remaining retry budget is never used. Two sites then feed a possibly empty value into an arithmetic comparison.
cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh#L364-L383: append|| trueto theenroll pendingsubstitution at line 374, and apply the same guard plus a[[ "$CONNECTED_CLIENTS" =~ ^[0-9]+$ ]]check to theenroll statussubstitution at lines 527-529.cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh#L431-L432: append|| trueto theenroll pendingsubstitution so the loop continues on a transient SSH failure.cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh#L447-L452: append|| trueto theenroll statussubstitution and guard(( CONNECTED >= 1 ))with[[ "$CONNECTED" =~ ^[0-9]+$ ]].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh` around lines 364 - 383, Update the retry loops to tolerate transient command failures under set -e. In cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh lines 364-383, guard the enroll pending substitution and enroll status substitution with || true, and only perform arithmetic after validating CONNECTED_CLIENTS with [[ "$CONNECTED_CLIENTS" =~ ^[0-9]+$ ]]. In cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh lines 431-432, guard enroll pending with || true; at lines 447-452, guard enroll status similarly and validate CONNECTED with [[ "$CONNECTED" =~ ^[0-9]+$ ]] before comparing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux-tui/apps/macos/NativeMuxDemo/README.md`:
- Around line 23-27: Remove the branch-specific
worktrees/feat-cmux-tui-swift-frontend/ prefix from every documented command in
the NativeMuxDemo README, including the run-demo.sh references and the commands
at the other noted locations. Use repository-relative paths beginning with
cmux-tui/apps/macos/NativeMuxDemo/ so the commands work from a normal checkout.
In `@cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh`:
- Around line 466-478: Reduce or remove the per-iteration remote polling in the
idle wait loop around remote_command. Prefer relying on the existing local kill
-0 checks for APP_PID and SSH_DAEMON_PID; if remote enroll connections state
must remain, poll it at a substantially longer interval rather than every 250
ms, while preserving the existing exit and daemon-failure behavior.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swift`:
- Around line 33-35: Update the lock icon in the BrowserView URL display to
derive its system image from browser.url’s scheme: use the secure lock only for
HTTPS and an appropriate non-secure icon for HTTP or other schemes, preserving
the existing styling.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/DemoWindowPlacement.swift`:
- Around line 61-62: Update the layout persistence flow around JSONEncoder and
data.write in DemoWindowPlacement to log encoding and file-write failures with
Logger, while preserving the current early-return behavior and atomic write
option.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendModel.swift`:
- Around line 318-345: Update setViewportWidth to convert columns into a
fractional viewport width using Double(columns) divided by screenWidth, while
preserving the existing clamping and mutation flow as appropriate for the
0.1–1.0 pane.viewport_width.set contract.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendService.swift`:
- Around line 277-313: Bound the event accumulation in drainRenderEvents by
introducing a maximum batch size and stopping once result reaches that limit,
returning the collected events so the caller can drain remaining queued events
on a later invocation. Preserve the existing event-copying and unknown-event
handling behavior.
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Localization.swift`:
- Around line 3-12: Replace the static-only L10n namespace with a constructable
localization dependency that provides text and format operations as instance
methods. Inject this dependency into the view or controller owners that render
user-facing strings, and update their calls to use the injected instance instead
of L10n’s static APIs; avoid introducing global or singleton state.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swift`:
- Around line 136-148: Update NativeTerminalModel.resize to use a stored,
cancellable model-owned resize Task instead of creating an untracked Task per
geometry change. Coalesce updates so the operation submits the newest pending
geometry after any in-flight handle.resize completes, while preserving rejection
handling for the latest request. Cancel and clear this operation during
shutdownAndWait().
- Around line 15-20: Move the top-level terminalGeometry function onto the
TerminalGeometry type as an initializer or static member, preserving its
existing width/height calculations and clamping behavior. Update every caller to
use the new TerminalGeometry-owned API and remove the internal free function.
In `@cmux-tui/apps/macos/NativeMuxDemo/test-remote-lifecycle-ready.sh`:
- Around line 30-41: Increase the startup-attempt argument in the
cmux_wait_for_remote_demo_ready call for the first lifecycle case so it
comfortably exceeds the delay between the startup message and “Ready.” Keep the
long-transfer budget and all other arguments unchanged, preserving the test’s
verification that the transfer does not consume the startup budget.
In `@cmux-tui/apps/macos/NativeMuxDemo/verify-demo-lifecycle.sh`:
- Around line 95-99: Update the APP_PID discovery loop around new_pids to avoid
head -1 terminating the pipe early under pipefail; read only the first PID while
allowing new_pids to finish successfully, preserving the existing retry and
break behavior.
In `@cmux-tui/crates/cmux-terminal-client/include/cmux_terminal_client.h`:
- Around line 84-88: Remove constness from the consuming render-event accessor
by changing cmux_frontend_terminal_copy_next_render_event to accept a mutable
terminal pointer in both the C header and the Rust extern-C definition in
frontend.rs. Update any related declarations or call sites as needed while
preserving the existing queue-consumption behavior.
In `@cmux-tui/crates/cmux-terminal-client/src/frontend.rs`:
- Around line 845-855: Update cmux_frontend_client_disconnect around the
cmux-frontend-disconnect cleanup thread so it retains the thread’s JoinHandle
and waits for it to finish before returning, while preserving the separate
thread needed to drop the Runtime safely. Ensure completion is observed only
after control.close, multiplexer.shutdown, connection.close, and provider.close
have all completed.
- Around line 357-368: Update the open_control_stream handshake invoked by
connect_transport to accept and enforce the timeout already supplied to
frontend_connect, including the loop waiting for all three Opened lanes. Ensure
the timeout propagates through the runtime.block_on call and returns the
existing connection error/cleanup path instead of blocking indefinitely.
- Around line 690-698: Update the encode flow around encode_key so the mutex
guard is released before matching the result and handling errors. Bind the
encode_key(chord, repeat) result first, then lock terminal.state only in the Err
branch to set status, notify updates, and return false; preserve the existing
success and enqueue behavior.
In `@cmux-tui/crates/cmux-terminal-client/src/lib.rs`:
- Around line 296-329: Update push_native_render_event so the eligible adjacent
Bytes events are coalesced before enforcing the event-count limit, while still
enforcing the byte budget for both coalesced and newly queued events. Preserve
queue clearing and false-return behavior when either limit is exceeded. Add unit
tests covering coalescing, byte and count limits, and prepare_handshake
resetting both native_render_events and native_render_event_bytes.
---
Duplicate comments:
In `@cmux-tui/apps/macos/NativeMuxDemo/README.md`:
- Around line 1-86: Add a Japanese README alongside the existing NativeMuxDemo
documentation that mirrors all launch, lifecycle, remote-demo, cleanup, command,
and safety details in README.md, using the same commands without alteration.
Update the package entry point to link both the English and Japanese documents,
and ensure the Japanese document is included through the app’s existing
ja.lproj/catalog structure where applicable.
In `@cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh`:
- Around line 364-383: Update the retry loops to tolerate transient command
failures under set -e. In cmux-tui/apps/macos/NativeMuxDemo/run-demo.sh lines
364-383, guard the enroll pending substitution and enroll status substitution
with || true, and only perform arithmetic after validating CONNECTED_CLIENTS
with [[ "$CONNECTED_CLIENTS" =~ ^[0-9]+$ ]]. In
cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh lines 431-432, guard enroll
pending with || true; at lines 447-452, guard enroll status similarly and
validate CONNECTED with [[ "$CONNECTED" =~ ^[0-9]+$ ]] before comparing it.
In `@cmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.sh`:
- Around line 256-273: Update the EXIT-trap cleanup function around cleanup so
failures recorded in exit_status override the final process status, rather than
relying on return. Preserve the existing daemon-alive and remote-removal checks,
but explicitly terminate or propagate exit_status from the trap when it is
nonzero so verify-remote-demo lifecycle checks observe the cleanup failure.
In `@cmux-tui/apps/macos/NativeMuxDemo/test-remote-command.sh`:
- Around line 40-55: Remove the unreachable fallback branch beginning at the
else clause, including its sed/eval extraction, test environment setup, and
remote_under_test wrapper. Keep the sourced remote-command.sh path as the sole
setup in the test script, and remove the now-unused RUN_REMOTE_DEMO variable.
In `@cmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.sh`:
- Around line 10-24: Move the argument validation in
verify-remote-demo-lifecycle.sh before the TEST_ROOT assignment and any
temporary-directory setup. Preserve the existing usage message and exit status,
while ensuring invalid arguments cannot create an uncleaned temporary directory.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d664edff-be3c-45bc-92fc-20058a680059
⛔ Files ignored due to path filters (1)
cmux-tui/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
cmux-tui/Cargo.tomlcmux-tui/apps/macos/NativeMuxDemo/Package.swiftcmux-tui/apps/macos/NativeMuxDemo/README.mdcmux-tui/apps/macos/NativeMuxDemo/Sources/CCmuxTerminal/cmux_terminal_client.hcmux-tui/apps/macos/NativeMuxDemo/Sources/CCmuxTerminal/module.modulemapcmux-tui/apps/macos/NativeMuxDemo/Sources/GhosttyKit/module.modulemapcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/DemoWindowPlacement.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendService.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRemoteSurfaceView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRuntime.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/JSONValue.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/LayoutNode.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/LayoutViews.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Localization.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeMuxDemoApp.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/PaneView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/ResourceSnapshot.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/en.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/ja.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/RootView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/SpacesBar.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/TerminalView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/WorkspaceSidebar.swiftcmux-tui/apps/macos/NativeMuxDemo/Support/Info.plistcmux-tui/apps/macos/NativeMuxDemo/Support/en.lproj/InfoPlist.stringscmux-tui/apps/macos/NativeMuxDemo/Support/ja.lproj/InfoPlist.stringscmux-tui/apps/macos/NativeMuxDemo/Tests/NativeMuxDemoTests/NativeMuxDemoTests.swiftcmux-tui/apps/macos/NativeMuxDemo/launch-ghostty-client.shcmux-tui/apps/macos/NativeMuxDemo/remote-command.shcmux-tui/apps/macos/NativeMuxDemo/remote-daemon-owner.shcmux-tui/apps/macos/NativeMuxDemo/remote-lifecycle-ready.shcmux-tui/apps/macos/NativeMuxDemo/run-demo.shcmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.shcmux-tui/apps/macos/NativeMuxDemo/test-remote-command.shcmux-tui/apps/macos/NativeMuxDemo/test-remote-lifecycle-ready.shcmux-tui/apps/macos/NativeMuxDemo/verify-demo-lifecycle.shcmux-tui/apps/macos/NativeMuxDemo/verify-remote-demo-lifecycle.shcmux-tui/bindings/cpp/.cmux-resource-api.jsoncmux-tui/bindings/go/.cmux-resource-api.jsoncmux-tui/bindings/java/.cmux-resource-api.jsoncmux-tui/bindings/python/.cmux-resource-api.jsoncmux-tui/bindings/rust/.cmux-resource-api.jsoncmux-tui/bindings/typescript/.cmux-resource-api.jsoncmux-tui/bindings/zig/.cmux-resource-api.jsoncmux-tui/crates/cmux-remote/Cargo.tomlcmux-tui/crates/cmux-remote/src/lib.rscmux-tui/crates/cmux-remote/src/mux_codec.rscmux-tui/crates/cmux-remote/src/service.rscmux-tui/crates/cmux-terminal-client/Cargo.tomlcmux-tui/crates/cmux-terminal-client/include/cmux_terminal_client.hcmux-tui/crates/cmux-terminal-client/src/frontend.rscmux-tui/crates/cmux-terminal-client/src/lib.rscmux-tui/crates/cmux-terminal-host-protocol/Cargo.tomlcmux-tui/crates/cmux-terminal-host-protocol/src/lib.rscmux-tui/crates/cmux-tui-core/Cargo.tomlcmux-tui/crates/cmux-tui-core/src/lib.rscmux-tui/crates/cmux-tui-core/src/resource_router.rscmux-tui/crates/cmux-tui-core/src/surface.rscmux-tui/crates/cmux-tui-core/src/terminal_host_protocol.rscmux-tui/scripts/check-spec-inventory.pycmux-tui/scripts/test_check_spec_inventory.pycmux-tui/spec/resource-operations-v2.jsoncmux-tui/spec/terminal-host.md
lawrencecchen
force-pushed
the
feat-cmux-tui-swift-frontend
branch
from
August 7, 2026 08:58
dedbbf8 to
e40d1ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
cmux-tui/crates/cmux-terminal-client/src/frontend.rs (1)
379-383: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClose the control stream before tearing down the transport.
streamis already open on Line 363. This failure path shuts down the transport but does not callstream.close().await. Close the service stream first so the daemon receives a clean service closure before multiplexer shutdown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/crates/cmux-terminal-client/src/frontend.rs` around lines 379 - 383, Update the shutdown flow in the runtime.block_on cleanup block to call stream.close().await before multiplexer.shutdown() and connection.close(). Preserve the existing provider.close() cleanup afterward so the control stream is closed before transport teardown.cmux-tui/crates/cmux-terminal-client/src/lib.rs (1)
452-457: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the complete Kitty state in the native Reset event.
The text renderer restores
snapshot.replay,snapshot.kitty_image_aliases, andsnapshot.kitty_stateon Lines 429-434. The native Reset event forwards onlysnapshot.replay.Extend the native Reset payload and its Swift consumer so they restore the aliases and Kitty replay state with the VT replay. Add an image-containing snapshot regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmux-tui/crates/cmux-terminal-client/src/lib.rs` around lines 452 - 457, Update the native Reset flow around continue_after_native_event to carry snapshot.replay, snapshot.kitty_image_aliases, and snapshot.kitty_state, and update the corresponding Swift consumer to restore all three components like the text renderer. Add a regression test using a snapshot containing an image that verifies aliases and Kitty replay state survive Reset.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swift`:
- Line 33: Update the security-icon selection in BrowserView to normalize the
URL scheme before checking whether it is HTTPS. Parse the scheme and compare its
lowercased value so mixed-case HTTPS URLs select lock.fill, while non-HTTPS URLs
continue selecting globe.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swift`:
- Around line 144-154: Update the resizeTask closure around handle.resize so a
cancelled task returns immediately after the await, before updating errorMessage
or clearing state. When cleanup runs, clear resizeTask only if it still refers
to the active non-cancelled task, preventing an older task from clearing a newer
replacement.
In
`@cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/WorkspaceSidebar.swift`:
- Around line 62-66: Precompute and maintain a screenCountByWorkspaceID index in
ResourceSnapshot whenever screens are decoded or refreshed, rather than
filtering and sorting through screens for each workspace row. Update the
workspace sidebar row around snapshot.screens(in:) to read the workspace’s count
from this index while preserving the existing singular/plural formatting and
zero-count behavior.
---
Duplicate comments:
In `@cmux-tui/crates/cmux-terminal-client/src/frontend.rs`:
- Around line 379-383: Update the shutdown flow in the runtime.block_on cleanup
block to call stream.close().await before multiplexer.shutdown() and
connection.close(). Preserve the existing provider.close() cleanup afterward so
the control stream is closed before transport teardown.
In `@cmux-tui/crates/cmux-terminal-client/src/lib.rs`:
- Around line 452-457: Update the native Reset flow around
continue_after_native_event to carry snapshot.replay,
snapshot.kitty_image_aliases, and snapshot.kitty_state, and update the
corresponding Swift consumer to restore all three components like the text
renderer. Add a regression test using a snapshot containing an image that
verifies aliases and Kitty replay state survive Reset.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a8c51f5-f60e-4e34-b885-f73ab038d66b
📒 Files selected for processing (12)
cmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/BrowserView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/FrontendModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/GhosttyRemoteSurfaceView.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/NativeTerminalModel.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/en.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/Resources/ja.lproj/Localizable.stringscmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/SpacesBar.swiftcmux-tui/apps/macos/NativeMuxDemo/Sources/NativeMuxDemo/WorkspaceSidebar.swiftcmux-tui/apps/macos/NativeMuxDemo/run-demo.shcmux-tui/apps/macos/NativeMuxDemo/run-remote-demo.shcmux-tui/crates/cmux-terminal-client/src/frontend.rscmux-tui/crates/cmux-terminal-client/src/lib.rs
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This was referenced Aug 19, 2026
Collaborator
|
Closing this superseded NativeMuxDemo stack. Head |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Source-first successor to closed stacked PR 9459 after PR 9634 landed. Restores native renderer event queue and canonical VT color transitions, guards pressure input, and validates required invitation errors.
Note
High Risk
Large cross-language surface (Rust C ABI, Iroh transport, terminal host protocol, AppKit/Ghostty) with new CI paths; mistakes could affect remote sessions, PTY lifecycle, or test gating—not a small UI-only change.
Overview
Introduces NativeMuxDemo, a standalone macOS SwiftUI client that talks to the cmux remote daemon over Iroh and renders terminals locally with GhosttyKit (Metal/libghostty) instead of parsing PTY bytes in Rust
ghostty-vt.The Swift app wires workspaces, spaces, niri-style viewport columns, splits/stacks, terminal and browser panes, optimistic focus, session event streams, and per-pane native terminal surfaces via a new C ABI from
cmux-terminal-client(native-rendererfeature).Protocol and workspace: Adds
cmux-terminal-host-protocoland routescmux-tui-core/cmux-terminal-clientthrough it for shared host framing and CMNR reset payloads. CI full mode now runs workspace-wide isolated Rust tests (replacing core-only isolation), doctests, buildscmux-terminal-clientwithnative-renderer, runs Swift tests, and anative-frontendjob gates hosted verification.Tooling: Demo launchers (
run-demo.sh, remote lifecycle scripts), Ghostty side-by-side client launcher, and EN/JA localization ship with the app bundle undercmux-tui/target/native-mux-demo/.Reviewed by Cursor Bugbot for commit d40d59a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a native macOS SwiftUI frontend that renders terminals locally with
GhosttyKit, extracts terminal‑host framing intocmux-terminal-host-protocolv4, and restores an ordered, backpressured native render queue with a safe C ABI for Swift. CI now runs workspace‑wide isolated Rust (incl. doctests) and Swift tests, gates a hostednative-frontendjob, and the remote demo readiness flow is phase‑aware with separate transfer and startup budgets.New Features
NativeMuxDemo: localGhosttyKitterminals; ordered/backpressured render queue with canonical color transitions; input‑epoch guards; coalesced resizes; one active terminal per visible pane; per‑handle serialized Swift FFI; runs frontend requests off the actor executor; recovers event gaps; EN/JA; optional side‑by‑side Ghostty launcher; Swift tests; local/remote lifecycle scripts; closes streams before teardown; phase‑aware remote readiness budgets.cmux-terminal-host-protocolv4 (extracted from core);cmux-remote-protocol::RESOURCE_PROTOCOL;cmux-remoteexposes publicmux_codecand gates daemon internals behind adaemon-servicesfeature (default);cmux-terminal-clientships a native frontend C ABI and Swift modulemap, preserves ordered resource envelopes, restores bounded Kitty replay/snapshots, retains leased render bytes/events across resync and backpressure accounting, bounds and drains resource batches, reuses shared transport enrollment, serializes per‑handle FFI, and closes streams before shutdown.pane split --viewport-widthrequires--rightand validates 0.1–1.0 with localized errors; spec clarifies bounded clear‑history VT replay; CI uses a new workspace‑wide isolated test runner and runs doctests, Swift tests, and a hostednative-frontendgate.Migration
cmux-remote: defaults now includedaemon-services. For client‑only builds, disable default features and enableiroh-transport.cmux-terminal-host-protocol; read the control protocol viacmux-remote-protocol::RESOURCE_PROTOCOL.cmux-terminal-clientfeatures: defaulttext-renderer; enablenative-rendererto skipghostty-vt. Swift app: includeGhosttyKit.xcframeworkinPackage.swift; useapps/macos/NativeMuxDemo/run-demo.shorrun-remote-demo.sh.Written for commit 8318e49. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests