Repository navigation
cmux-tui protocol v7: render-state attach, server-paged scrollback, tree deltas + cmux-lite Swift frontend - #8031
Merged
Merged
Conversation
…ping Render mode ships the server's authoritative terminal state to rich clients: render-state snapshots and damage-coalesced render-delta events carry styled maximal runs (plain UTF-8, resolved RGB, explicit underline variants), full cursor visuals, and scrollback paging via read-scrollback. Byte mode stays the default and exact v6 behavior. Tree deltas are opt-in per subscription (tree_events, default coarse) so v6 subscribers never go stale. Every event names its subject; colors-changed gains surface. send gains paste (server-side bracketed-paste iff mode 2004). One consolidated Sizing section replaces per-command prose. Two engine-work items recorded: wide-cell spacer discrimination, non-mutating styled scrollback access.
RenderState::build_frame captures a consume-once damage snapshot (RenderFrame: seq, size, cursor visuals, dirty rows, immutable styled rows) that one producer fans out to the TUI and N render attachments; walk_rows behavior is unchanged. styled_history_rows/history_rows read scrollback via untracked history grid refs without touching the viewport or damage. rows_to_runs assembles maximal spec-shaped runs: resolved RGB with None for defaults, the render.md attrs bit table, underline as an enum, wide tails folded into their lead with width_hint. Upstream C API affordances (GHOSTTY_CELL_DATA_WIDE, GHOSTTY_POINT_TAG_HISTORY) covered both open engine questions; the submodule is untouched.
New macOS app under frontends/swift/CmuxLite: Swift 6 package + xcodegen project, GhosttyKit (libghostty-spm, pinned) rendering a byte-mode attach over WebSocket with the auth preamble, vt-state/output/resized replay handling, keystrokes via send, debounced size-equal-guarded resize-surface, minimal workspace sidebar, en+ja strings. Live-smoked against a running session end to end. Review fix: the v6 resized replay travels in `data` (the spec's draft `replay` spelling is corrected and verified against server.rs; decoder accepts both).
attach-surface mode:"render" streams render-state snapshots and damage-coalesced render-delta events from a per-surface frame producer: PTY output schedules a bounded 8ms coalesce, build_frame runs once per generation, and the immutable frame fans out to the local TUI and every render attachment; with no render taps the producer skips frame construction entirely, so byte-only servers keep their v6 cost. Snapshot and tap register under the same terminal lock as byte attach (no gap, no duplication). read-scrollback pages styled history. subscribe tree_events:"deltas" opts into 11 entity events built inside the validating state-lock scopes (TreeChanged fallback rather than panics on concurrent closes); coarse subscribers see exact v6 behavior. colors-changed names its surface. send paste:true wraps in bracketed paste iff mode 2004. The TUI's palette rendering keeps host-themed ANSI colors unless overridden. PROTOCOL_VERSION 7.
attachSurface gains mode:"render" (client-gated on cached identify.protocol >= 7, throws before sending) yielding typed render-state/render-delta events with no base64 on the render path; readScrollback pages styled history; subscribe treeEvents:"deltas" types the 11 entity events; colors-changed surface optional for v6 servers; send gains paste. Wire shapes asserted against the server's actual serialization in tests (18/18).
…r v6 Against protocol >= 7 the pane renders the server's styled runs directly: a pure row model applies render-state snapshots and dirty-row deltas, DOM rows of spans carry resolved RGB, attrs, and underline variants, and the cursor is an element at the server's cell coordinates with its real style/blink (hollow when unfocused). Input goes through a hidden textarea with IME composition buffering, a key-encoding table through F24, and paste:true. Scrollback is real: server-paged history with a bounded bidirectional window (growth keeps your reading position; shrink/resize invalidate), a back-to-live control, and load-newer to the live boundary. Native DOM selection works because the text is text. The xterm byte path remains for protocol-6 servers.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces cmux-tui protocol v7, adding render-mode surface attachment, tree lifecycle delta events, styled scrollback reading, bracketed paste, and a pairing-based WebSocket authentication flow. It updates the Rust server/TUI, ghostty-vt rendering engine, TypeScript SDK, web frontend, and adds a new native Swift "CmuxLite" AppKit frontend, plus spec/doc/CI updates. ChangesRust core, TUI, and rendering engine
Estimated code review effort: 5 (Critical) | ~120 minutes TypeScript SDK
Web frontend
Swift CmuxLite frontend
Specs, docs, and CI
Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebSocketServer
participant PairingBroker
participant TrustedUnixClient
Browser->>WebSocketServer: pair request
WebSocketServer->>PairingBroker: begin(peer)
PairingBroker-->>WebSocketServer: PairingChallenge
WebSocketServer->>TrustedUnixClient: pairing-requested event
TrustedUnixClient->>WebSocketServer: PairingResponse(approve)
WebSocketServer->>PairingBroker: respond(id, approve)
PairingBroker-->>Browser: PairingDecision credential
sequenceDiagram
participant Client
participant Server
participant Surface
Client->>Server: attach-surface mode:"render"
Server->>Surface: attach_render_stream()
Surface-->>Client: render-state (full snapshot)
loop terminal updates
Surface-->>Client: render-delta / scroll-changed
end
Client->>Server: detach
Server-->>Client: detached
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (8 errors, 2 warnings)
✅ Passed checks (15 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 |
Contributor
Greptile SummaryThis PR makes server-rendered terminal state available to rich clients. The main changes are:
Confidence Score: 4/5The Swift event queue can grow without limit under sustained terminal output. The web scrollback and IME paths also need contained fixes.
CmuxProtocolClient.swift, scrollback.ts, and styles.css. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PTY
participant Server as cmux-tui server
participant Client as Protocol client
participant UI as Web or Swift frontend
PTY->>Server: terminal bytes
Server->>Server: update emulator and build frame
Server-->>Client: render-state or render-delta
Client-->>UI: decoded attach event
UI->>UI: apply rows, cursor, and colors
UI->>Server: read-scrollback(start, count)
Server-->>UI: styled history page
UI->>Server: send input or resize
%%{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 PTY
participant Server as cmux-tui server
participant Client as Protocol client
participant UI as Web or Swift frontend
PTY->>Server: terminal bytes
Server->>Server: update emulator and build frame
Server-->>Client: render-state or render-delta
Client-->>UI: decoded attach event
UI->>UI: apply rows, cursor, and colors
UI->>Server: read-scrollback(start, count)
Server-->>UI: styled history page
UI->>Server: send input or resize
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// - Parameter transport: The full-duplex transport implementation. | ||
| public init(transport: any CmuxTransport) { | ||
| self.transport = transport | ||
| let pair = AsyncStream<CmuxAttachEvent>.makeStream(bufferingPolicy: .unbounded) |
Contributor
There was a problem hiding this comment.
The event stream uses unbounded buffering while the WebSocket receive loop can yield terminal output faster than the AppKit consumer renders it. Sustained high-volume PTY output therefore grows the queue and process memory without limit instead of applying backpressure or coalescing adjacent output events.
Comment on lines
+83
to
+91
| } | ||
|
|
||
| export function mergeScrollbackPage( | ||
| window: ScrollbackWindow, | ||
| page: ReadScrollbackResult, | ||
| ): ScrollbackWindow { | ||
| const existing = page.total < window.total ? [] : window.rows; | ||
| const byIndex = new Map<number, RenderRow>(); | ||
| for (const row of existing) byIndex.set(row.row, row); |
Contributor
There was a problem hiding this comment.
When pagination pushes the cache past maxRows, trimming can remove the row used as the current scroll anchor. scrollbackAnchorDelta then returns zero because it cannot find that row, so loading another history page can move the reader to the wrong position instead of preserving the visible content.
… webgl Every accepted WS stream (and control clones, CDP sockets) sets nodelay so keystroke-sized frames stop hitting Nagle. Headless servers seed mux defaults from the user's ghostty config before any surface spawns: fg/bg, selection colors, cursor style/blink, and palette entries through a thin ghostty-vt color-API wrapper, so render-mode colors match the user's terminal. The web byte path loads @xterm/addon-webgl with silent fallback; the render path coalesces deltas into one reducer publication per animation frame. Latency audit: the 8ms producer is leading-edge; remaining timers are teardown/liveness only.
Dark #0c0c0c chrome matching the TUI: dense rail-accented workspace sidebar with dim subtitles, slim per-pane tab bar (select-tab/new-tab, shared server state), bottom screens strip with chips and session badge, terminal filling its pane. Navigation is client-local. The reported blinking is fixed at both roots: grid size derives only from final laid-out bounds with real cell metrics (floored, debounced, echo-guarded, never mid-replay — pure decision function with tests) and resized replays feed the one persistent terminal instance instead of rebuilding the view.
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…h rows New smoke benchmark (200-sample keystroke echo RTT through the app's own client path) exposed p50=101.7ms: the server's ws loop drains outbound only between 100ms blocking reads. Client-side: NWConnection WebSocket transport with TCP nodelay and response-bound wake frames (URLSession fallback) takes p50 to 0.64ms / p95 0.96ms; input pipelines without awaiting responses; output feeds GhosttyKit immediately. Tabs render as numbered chips with title tooltips and no intrinsic padding; sidebar rows are flush full-width with the rail at the edge; screens strip loses its leading padding.
UnixSocketTransport speaks the server's native newline-delimited JSON over NWConnection (no auth preamble, no Nagle to fight): --socket/ --session mirror the server's socket path convention, no flags selects the sole live socket, --url keeps WebSocket (wake frames stay WS-only) as the web-parity path. Live benchmark: unix p50 0.076ms / ws 0.390ms. GhosttyKit view config now applies the user's ghostty theme, font-family, font-size, selection colors, and cursor style/blink defaults via one tested parser. Tabs keep compact content with 34pt hit targets.
The ws connection loop drained its outbound queue only between blocking reads with a 100ms poll timeout, so every event waited out the read — measured 96ms cross-connection delivery. Reads now wait for readability via poll() on a cloned fd OUTSIDE the shared-socket mutex with a bounded 5ms locked read, while a dedicated writer thread blocks on the outbound channel and sends the moment an event is queued: 117µs delivery, 800x. Clean-close semantics preserved (final drain then Close). Unix was never affected (LineSink writes immediately). Regression test asserts <50ms cross-connection delivery and fails deterministically on the old code.
Recursive multi-pane rendering of the layout tree with one byte attachment per visible pane, local focus, draggable optimistic dividers committing set-ratio, and clean attachment lifecycle. One shared action table drives the shortcut map (cmd-D/shift-D splits, cmd-T/W tabs, cmd-1..9 tabs, ctrl-1..9 screens, cmd-alt-arrows local pane focus, cmd-ctrl-arrows ratio nudges); unmatched keys reach the terminal. Every highlight state audited onto the TUI palette. Socket discovery now mirrors the server's runtime-base precedence (XDG_RUNTIME_DIR, TMPDIR, /tmp). en+ja including the shortcut README.
This comment has been minimized.
This comment has been minimized.
Headless color seeding now prefers a bounded `ghostty +show-config` invocation (env/PATH/app-bundle binary discovery) and consumes its fully resolved output, inheriting ghostty's semantics permanently; the file parser fallback makes theme lines fail-soft so the last LOADABLE theme wins, matching observed ghostty behavior (a later not-installed theme is ignored instead of stomping the palette). Also restores the cmux-tui package block a broken earlier lockfile commit dropped.
Theme resolution consumes `ghostty +show-config` (fail-soft file parser fallback) so the app shows exactly what ghostty shows (dark Monokai, not the not-installed later theme line). Tab clicks select again (the mouse monitor rebuilt buttons before their action fired); dot glyphs removed. Every surface-creating command sends the initiating pane's grid so PTYs spawn at final size — no post-spawn SIGWINCH, no zsh %% artifact.
Root cause of blurry glyphs and the offset/spread window: the app stretched the terminal view's layout to the pane, reflowing foreign grids at the wrong geometry (GhosttyKit's backing-scale handling was already correct). The terminal view is now pinned to grid x native cell pixels / backing scale, top-left, clipped inside a terminal-background container, with a localized "sized by another client, type to take over" hint. Geometry regression tests + live 2x proof (drawable math, foreign 60x18 letterbox, keystroke claim).
…cy, zig fallback The bracketed-paste test now waits (bounded) for DEC 2004 to be applied through the async reader before pasting instead of racing it, and dumps terminal state on failure. The history-eviction test asserts invariants (oldest advanced, retained within cap plus one terminal page) at 20k rows instead of an allocator-sensitive exact count. The Package.resolved policy narrowly allows Swift frontends' checked-in Xcode project lockfiles, mirroring the root project allowance. install-zig-ci gains a SHA-256 verified fallback when minisign is unavailable plus mirror resilience.
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. |
macOS od prints hex pairs double-spaced, so the single-spaced needle never matched even though the diagnostic dump showed the bracketed bytes present; both the bracketed and raw matchers now compare with whitespace collapsed. (Completes the previous commit, whose author process was killed between pushing and writing this final hunk.)
The paste test's hex assertions collapse whitespace before matching (od pads pairs differently on macOS), and the conformance fixtures plus the python e2e protocol range accept the v7 identify. The earlier zig 404 was transient mirror infra; downloads succeed with the added fallback.
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cmux-tui/spec/events.md (1)
716-734: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the v7
surfacerequirement explicit in the schema.The payload declares
surface?:Id, but the following contract says protocol 7 requires an explicit surface id. A client with multiple attachments cannot safely route the event from this schema. Document a versioned shape: omitsurfacefor v6, requiresurface:Idfor v7.🤖 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/spec/events.md` around lines 716 - 734, Update the colors-changed payload schema to document versioned shapes: protocol v6 omits surface, while protocol v7 requires surface: Id. Keep the existing color and cursor fields unchanged, and make the v7 requirement explicit so clients can route events across multiple attachments.cmux-tui/frontends/web/src/components/TerminalPane.tsx (1)
121-127: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReplace raw transport errors with safe localized copy.
Both terminal implementations pass SDK/transport failures here, and
error.messageis later rendered verbatim. This may expose provider internals, request/session identifiers, or upstream payload details. Present localized product-level recovery text and retain sanitized diagnostics only in internal telemetry.As per coding guidelines, user-facing errors must not expose raw upstream messages or internal identifiers.
🤖 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/frontends/web/src/components/TerminalPane.tsx` around lines 121 - 127, Update reportError in TerminalPane to stop storing error.message for rendering; use the established localized, product-level recovery message instead, while routing any sanitized diagnostic details through internal telemetry if that mechanism already exists. Keep the existing client and surface association unchanged so only the displayed error content changes.Source: Coding guidelines
cmux-tui/crates/cmux-tui-core/src/server.rs (1)
555-604: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftBound WebSocket egress and make writer shutdown interruptible.
channel()is unbounded and socket writes have no normal deadline. A slow client can accumulate terminal frames until OOM; disconnect then drains the backlog andjoin()can hang indefinitely on a blocked write holding the WebSocket mutex.Use a bounded queue with an overflow/coalescing policy, finite write deadlines, and cancellation that interrupts blocked writes.
Also applies to: 1021-1030, 1081-1082
🤖 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-tui-core/src/server.rs` around lines 555 - 604, Update WebSocketSink and run_websocket_writer to use a bounded outbound queue with an explicit overflow/coalescing policy for terminal frames, preventing unbounded memory growth. Apply finite write deadlines before socket sends, and add cancellation that can interrupt both queued-message draining and blocked writes so writer shutdown and join complete promptly. Preserve delivery of already-queued responses where possible while ensuring disconnect cannot wait indefinitely.cmux-tui/crates/cmux-tui-core/src/mux.rs (1)
953-989: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPublish tree deltas in the same serialized operation as their mutations.
These paths release
statebeforeemit, so concurrent commands can publish deltas in reverse mutation order. The close paths are worse: they snapshot tabs and the delta, then reacquire the lock to remove them; a concurrently added tab can survive while aworkspace-closedorpane-closedevent is emitted.Use one ordered mutation/event owner. Collect surfaces to kill under that owner, then perform only the kills afterward.
Also applies to: 1011-1121, 1150-1272, 1303-1547, 1558-1795, 1987-2074
🤖 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-tui-core/src/mux.rs` around lines 953 - 989, Serialize each workspace, pane, and screen mutation together with its corresponding TreeDelta publication using a single ordered mutation/event owner, including the paths around workspace creation and the listed close/open operations. For close paths, snapshot the entities and collect surfaces to kill while holding that owner, remove the entities and emit their deltas before releasing it, then perform only the collected surface kills afterward. Ensure concurrent commands cannot reorder mutation events or allow newly added tabs/panes to survive a close delta.
🤖 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/bindings/typescript/src/client.ts`:
- Around line 543-553: The matchesAttachEvent filter currently rejects unknown
surface-scoped events, causing shared and dedicated transports to differ. Update
matchesAttachEvent to accept unrecognized events whose surface matches, while
still excluding known events incompatible with the requested bytes or render
mode; alternatively remove UnknownEvent from the promised RenderAttachEvent
return type and add coverage for both transport modes.
- Around line 355-357: Update the bytes selection in the send request flow to
use options.base64 only when it is defined; otherwise retain the computed
legacyBytes value. Preserve explicit base64 values while ensuring an options
object with base64: undefined still sends the legacy bytes.
In `@cmux-tui/bindings/typescript/src/websocket-transport.ts`:
- Around line 187-193: Update the pairing handling around the credential branch
in the websocket transport so internal state and pending-request flushing
complete independently of onPairingCredential: preserve queue order when the
callback reentrantly calls send(), and ensure a thrown callback does not strand
queued requests. Apply the same protection to the rejection callback path around
the close handling so close handlers always run even if the consumer callback
throws.
In `@cmux-tui/crates/cmux-tui-core/src/mux.rs`:
- Around line 540-605: Serialize size reconciliation in
resize_surface_for_client, remove_surface_size_client, and remove_size_client so
map updates and the corresponding resize_surface application cannot interleave
with stale calculations. Hold the client_surface_sizes reconciliation lock
through effective-size calculation and resize application, or recheck/version
the map immediately before applying, ensuring the PTY size always matches the
latest recorded viewers.
In `@cmux-tui/crates/cmux-tui-core/src/pairing.rs`:
- Around line 142-162: The respond method currently conflates missing requests
with response-channel delivery failures and stores credentials even when
delivery fails. Update PairingManager::respond and its callers to return a
structured outcome distinguishing not-found from delivery failure, ensure every
found request emits PairingResolved regardless of response delivery status, and
only retain a generated credential when the approved decision can be delivered
successfully.
In `@cmux-tui/crates/cmux-tui-core/src/server.rs`:
- Around line 1924-1944: Limit the requested page size in the
Command::ReadScrollback handler before calling styled_history_rows, using a
small protocol-defined maximum; reject or clamp counts above that limit while
preserving the existing u16 validation and response behavior.
- Around line 1398-1444: Update tree_entity_json to construct only the requested
workspace, screen, pane, or tab JSON directly from authoritative State data
instead of calling workspaces_json and scanning the materialized tree. Use
existing state indexes or single-pass lookups keyed by the provided id, preserve
the current Option<Value> behavior, and keep each TreeDeltaKind mapped to its
corresponding entity serializer.
- Around line 939-946: The connection-acceptance path must reject streams that
cannot be registered in thread_connections. Update the try_clone handling after
next_connection.fetch_add in the server loop to shut down the original stream
and continue when cloning fails; only spawn or proceed with connections after
successfully inserting the cloned stream into the tracking registry.
In `@cmux-tui/crates/cmux-tui-core/src/surface.rs`:
- Around line 192-196: Update RenderHub’s tap delivery and the related
attachment paths around RenderAttachFrame to use bounded queues with coalesced
latest-frame updates, preventing slow clients from retaining frames
indefinitely. Preserve lossless ordering for scroll-control events by separating
or otherwise reliably delivering those events without coalescing, and apply the
same behavior at the additional tap-creation sites.
- Around line 89-98: The DefaultColors update must serialize and apply every
field for remote sessions, not only fg/bg. Update
RemoteSession::set_default_colors and its server-side request handler to
transmit and reconstruct cursor, selection colors, cursor_style, cursor_blink,
and palette, and remove the early return for updates containing only cursor or
palette changes.
In `@cmux-tui/crates/cmux-tui-core/tests/pty.rs`:
- Around line 1037-1053: Update the output predicate in the PTY test to retain
decoded bytes from every “output” event in an accumulated buffer, then search
the combined buffer for “render-multi” instead of checking each chunk
independently. Keep consuming events until the marker is found or the existing
timeout expires.
- Around line 1069-1096: Update the PTY attach race test around the spawned
write and response-reading loop to coordinate the writer with the
snapshot/subscription boundary, ensuring the write occurs after attachment is
ready rather than merely racing by scheduling. After issuing the attach request,
drain events through a known post-write sequence barrier instead of stopping
once the response and first marker arrive, then assert exactly one matching
marker event after the barrier.
In `@cmux-tui/crates/cmux-tui-core/tests/websocket_transport.rs`:
- Around line 239-245: Update the synchronization loop around subscriber.read()
to handle an interrupted read by retrying, matching the existing EINTR-safe read
pattern used earlier in the test. Keep the current Pong, Ping, Text, and
unexpected-message behavior unchanged.
In `@cmux-tui/crates/cmux-tui/src/app.rs`:
- Around line 1445-1450: Update resolve_pairing so the session.respond_pairing
failure sets status_message to a safe product-level pairing error message
instead of error.to_string(). Keep the raw error internal by using the existing
logging or diagnostic mechanism if available, while preserving the pairing queue
advancement behavior.
In `@cmux-tui/crates/cmux-tui/src/config.rs`:
- Around line 1370-1395: The run_ghostty_show_config function must consume
Ghostty’s stdout while the process is still running to prevent the piped buffer
from blocking child termination. Update this function to read stdout
concurrently or redirect it appropriately, while preserving the existing
two-second deadline, cleanup on timeout, successful-exit check, and returned
output.
In `@cmux-tui/crates/cmux-tui/src/session/mod.rs`:
- Around line 560-569: Update the SurfaceHandle::Remote resize flow so
surface.set_reported_size(desired) runs only after session.request successfully
acknowledges the resize-surface command. Preserve the retry behavior by leaving
reported_size unchanged when the request fails or times out, rather than
discarding the result and caching the requested dimensions unconditionally.
In `@cmux-tui/crates/cmux-tui/src/ui/mod.rs`:
- Around line 41-44: Update browser_graphic_occluded() to treat the active
pairing_dialog.rect as an occluding region, alongside the existing menu and
prompt checks. Ensure browser graphics emitted by emit_graphics() cannot render
over the pairing dialog’s trusted challenge or buttons, while preserving current
behavior when no pairing dialog is open.
In `@cmux-tui/crates/cmux-tui/src/ui/overlay.rs`:
- Around line 53-56: Update the pairing dialog flow around the width/height
guard so approval cannot proceed while pairing_dialog is active but not visibly
rendered on terminals below 24×10. Either render a compact fallback within the
available screen or clear/reject the active approval request before returning,
while preserving normal rendering for supported terminal sizes.
- Around line 83-91: Update the label width calculation used by the overlay
layout, including the code behind label_width, to measure terminal display cells
rather than Unicode scalar count. Reuse the project’s existing terminal-width
utility or dependency so pairing/prompt button hitboxes, code centering, and
toast sizing remain aligned for wide glyphs.
In `@cmux-tui/crates/ghostty-vt/src/terminal.rs`:
- Around line 477-507: Bound oversized scrollback requests in
styled_history_rows before palette lookup or cell materialization, using a small
server-side row limit or byte budget and rejecting requests that exceed it.
Preserve the existing empty-result behavior for zero or out-of-range requests,
and ensure accepted pages still follow the current history-row traversal.
- Around line 718-731: Update the replay serialization around self.format(opts)
so the final cursor restoration does not clear DECAWM’s pending-wrap state;
remove the trailing CUP or place cursor restoration within the formatter before
its final sequence. Add a regression covering a replay ending at the exact line
width and verify the mirror’s next printable character wraps identically to the
source.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxGhosttyConfigurationResolver.swift`:
- Around line 62-109: Refactor showConfigOutput so each invocation owns its
Process, stdout draining, continuation, and structured timeout task locally
instead of using activeProcess, outputPipe, commandContinuation,
commandFinished, or untracked fire-and-forget Tasks. Ensure the timeout is
cancelled when the process completes, the process is terminated only on timeout,
and concurrent resolve calls are serialized or otherwise prevented from sharing
operation state.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxHoverButton.swift`:
- Line 19: The custom button styles disable the native focus ring without
providing replacement focus styling. In CmuxHoverButton.swift:19-19 and
CmuxTabButton.swift:13-13, retain the native focus ring or add equivalent
styling for the focused state so keyboard users can identify the active button.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxLiteApp.swift`:
- Around line 29-43: Update the `@MainActor` launch flow around
CmuxConnectionConfiguration.parse to delegate configuration discovery and both
file-system closures to an off-main service or detached operation. Have that
operation return only the parsed configuration, then consume it in launch while
preserving the existing arguments, environment, userID, and parsing behavior.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxLiteWindowController.swift`:
- Around line 192-228: Update runMutation to stop cancelling an in-flight
mutationTask and serialize mutation execution so workspace, tab, split, close,
and ratio commands cannot overlap or discard committed responses. Ensure every
completed authoritative CmuxFrontendStartup is applied, while retaining
rollback/error handling; only coalesce operations explicitly proven safe to
supersede.
- Around line 523-531: Replace raw Error stringification in showConnectionError
with a localized, product-safe failure message and log only sanitized
diagnostics; preserve the existing localized status presentation. Also update
CmuxLiteSmoke.swift lines 10-12 to emit localized safe stderr output instead of
interpolating the raw error.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxPalette.swift`:
- Around line 33-60: Remove the ambient palette state represented by
CmuxPalette.resolved, CmuxPalette.tui, and CmuxPalette.configure(with:). Make
palette construction instance-based from CmuxGhosttyViewConfiguration, then
inject or retain the resulting CmuxPalette through the owning
CmuxLiteWindowController, CmuxFrontendSession, or dependent views instead of
accessing it via global static state.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxRenderView.swift`:
- Around line 197-207: Optimize the terminal draw path around the run.text loop
and its padding loop to avoid constructing a String and attributed object for
every cell. Render protocol runs or prebuilt glyph batches using reserved
buffers, and replace the unbounded glyphAdvances/interpolated cache path with a
typed bounded cache reused across redraws. Apply the same change to the related
rendering block at the referenced additional location while preserving cell
spans, styling, and padding behavior.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxSplitDividerView.swift`:
- Around line 62-74: Replace the manual nextEvent loop in mouseDown(with:) with
native AppKit drag lifecycle callbacks so mouseDown only begins the drag and
returns immediately. Move point conversion and onDragChanged handling into the
appropriate dragged-event callback, invoke onDragEnded on mouse-up, and retain
onDragCancelled for cancellation while preserving the enabled/window/superview
guards.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxStateDump.swift`:
- Around line 4-42: Move CmuxStateDump’s SIGUSR1 registry and JSON dump writer
out of the production CmuxLiteApp Sources into the dedicated smoke/debug target,
preserving its configured-only behavior there. In
cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxStateDump.swift lines
4-42, remove the production harness; in
cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxTerminalHostViewController.swift
lines 103-116, remove verificationState() because the external harness must no
longer require a production controller accessor.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxTerminalHostViewController.swift`:
- Around line 84-87: Update the .detached(surface) handling in
CmuxTerminalHostViewController so that, after validating the detached surface
matches attachedSurface, it invokes a single detach action that clears the
authoritative attachment state, model, and history, cancels pending history
loading, and resets the rendered terminal. Ensure subsequent input cannot target
the detached surface.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxFrontendSession.swift`:
- Around line 38-40: Replace the unbounded buffering configured in
CmuxFrontendSession’s AsyncStream initialization with bounded snapshot
publication or backpressure. Ensure render deltas are coalesced under the
existing render-model owner rather than retained indefinitely, while preserving
explicit state ownership through eventStream and eventContinuation.
- Around line 52-90: Update the connection flow in CmuxFrontendSession so any
failure after controlClient.connect, including protocol rejection, subscribe,
reconcileAttachments, or snapshot errors, tears down all partially initialized
resources before rethrowing. Add scoped cleanup that closes the control client
and previously attached clients, while preserving normal successful connection
behavior and avoiding cleanup before the initial connect attempt succeeds.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPane.swift`:
- Around line 16-21: The CmuxPane decoder currently fabricates active and live
state when fields are missing or invalid. In CmuxPane.init(from:), decode
activeTab and dead strictly or preserve an explicit unknown state; in
CmuxWorkspaceTree.swift lines 119-132, make active-surface routing use only the
authoritative decoded state and reject dead or invalid panes/tabs rather than
falling back to the first live pane or PTY, failing closed when state is absent.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxProtocolClient.swift`:
- Around line 17-19: Update the CmuxProtocolClient event-stream initialization
around eventStream and eventContinuation to avoid AsyncStream bufferingPolicy
.unbounded for terminal render events. Use the existing or a new bounded,
backpressured/coalescing event owner that preserves render-delta ordering and
requests a fresh snapshot whenever coalescing cannot be done safely, while
retaining the public event delivery behavior.
- Around line 260-268: Update the request flow surrounding
withCheckedThrowingContinuation and pending[id] so cancellation removes the
pending entry and resumes the continuation with CancellationError. Replace the
unstructured fire-and-forget Task used for transport.send with caller-owned or
explicitly tracked cancellable work, ensuring send failures still invoke
failRequest(id:error:) and no request remains suspended after cancellation.
- Around line 24-37: Refactor CmuxProtocolClient around one explicit
disconnected/connecting/connected/terminal lifecycle state, updating connect,
close, receiveLoop, and pending-request handling together. Ensure failed
authentication or receiving closes the transport, cancels/clears the receive
task and pending requests, and cannot leave connected true without an active
receive loop. Make close transition to terminal after permanently finishing
eventStream, preventing reconnects and preserving atomic ownership invariants.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScreen.swift`:
- Around line 29-31: Make CmuxScreen decoding fail closed by requiring
authoritative layout and pane records instead of defaulting to .leaf(pane:
activePane) or an empty pane list; update CmuxScreen.swift at lines 29-31
accordingly. In CmuxScreenSnapshot.swift lines 41-58, resolve active-pane
identity only from the authoritative structured source and return nil or empty
compatibility state when unavailable; ensure both sites use the same reliable
source for pane identity and UI gating.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScrollbackWindow.swift`:
- Around line 73-85: The CmuxScrollbackReconciliation logic must not treat
nextTotal versus previousTotal as the cache-freshness signal. Update the
surrounding scrollback reconciliation API to consume an authoritative server
generation/base offset or typed rollover signal, and invalidate/rebuild retained
rows when that signal indicates eviction or rollover even when counts are
unchanged; preserve reuse only when freshness is confirmed.
- Around line 91-98: Update merging(_:) to avoid trapping when calculating the
absolute remote row index: validate or combine page.start and row.row using
overflow-safe arithmetic before applying the existing bounds check, and skip
rows whose calculation overflows or falls outside the valid page range.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/FallbackWebSocketTransport.swift`:
- Around line 20-31: The connect lifecycle in FallbackWebSocketTransport must
track the in-flight candidate so close() can cancel it and prevent stale
completion from installing a transport. Update the transport state machine
around connect(), close(), and activeTransport to explicitly represent
connecting, active, and closed states, retain the candidate during connection,
cancel and clear it on close, and reject any completion that no longer matches
the current valid state.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/NetworkWebSocketTransport.swift`:
- Around line 46-53: Update connect() and the NWConnection state handling around
connectContinuation so the .waiting state cannot leave the checked continuation
pending indefinitely. Route .waiting through the existing explicit
fallback/error path or enforce a bounded timeout, while preserving completion
for .ready, .failed, and .cancelled and ensuring cancellation still cancels the
connection.
In
`@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/UnixSocketTransport.swift`:
- Around line 92-97: Update UnixSocketTransport.receive() to enforce a protocol
maximum on the accumulated receiveBuffer, checking the pending frame size before
appending each receiveChunk(). Fail closed when the incomplete line exceeds that
maximum, while preserving nextBufferedMessage() handling for complete messages.
In `@cmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteSmoke/SmokeRunner.swift`:
- Around line 26-44: Update the argument validation in SmokeRunner’s CLI parsing
flow to emit localized messages through the project’s established localization
API instead of hardcoded strings. Add stable catalog keys with matching English
and Japanese entries for missing option values, invalid surface IDs,
non-positive iterations, and invalid transport values, preserving the current
error semantics and wording.
- Around line 227-243: Update the latency loop in SmokeRunner’s echo-sampling
flow and the corresponding waitForEcho call so each iteration waits for an echo
uniquely tied to its newly sent payload, rather than matching any character
already present in the accumulated screen. Use a unique per-iteration token or
compare against the model state captured before sending, while preserving the
existing timing and sample collection behavior.
In
`@cmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxWorkspaceTreeTests.swift`:
- Around line 18-24: Update selectedSurface() and its test
fallsBackToFirstLivePTYWhenActiveTabIsBrowser so the active tab remains
authoritative: when the selected tab is an unsupported browser, represent that
selection or return no terminal surface instead of falling back to surface 12.
Preserve selection of the active terminal tab and fail closed when no usable
authoritative terminal surface exists.
In `@cmux-tui/frontends/web/src/components/ConnectScreen.tsx`:
- Around line 60-65: Update ConnectScreen.tsx around the pairing status to
present pairing.expiresIn as a static approval-window duration rather than a
live countdown. Update the English copy in cmux-tui/frontends/web/src/i18n.ts
lines 9-10 and the equivalent Japanese copy at lines 78-79 so both use static
approval-window wording; keep all supported locales consistent.
- Line 67: Update the error alert in ConnectScreen to avoid rendering the raw
error value. Map pairing and transport/server failures to localized
product-level messages using the existing translation function t, with unknown
or unmapped failures falling back to t("unknownError"), while preserving the
alert’s role and display behavior.
- Around line 34-38: Update the submit handler in ConnectScreen so initial.token
is forwarded only when the submitted URL matches the original endpoint; when url
has been edited, omit the token or require an explicitly entered token instead.
Preserve the normalized URL submission and local-storage behavior.
In `@cmux-tui/frontends/web/src/components/StatusBar.tsx`:
- Around line 38-40: Constrain unbounded user-defined screen labels without
truncating their accessible full value: in
cmux-tui/frontends/web/src/components/StatusBar.tsx lines 38-40, add a title or
accessible label to the screen button; in cmux-tui/frontends/web/src/lib/tree.ts
lines 63-72, make no direct change and continue treating statusLabel as
unbounded input; in cmux-tui/frontends/web/src/styles.css lines 178-180, add a
maximum width, hidden overflow, and ellipsis text overflow to the screen-chip
styling.
In `@cmux-tui/frontends/web/src/components/TerminalPane.tsx`:
- Around line 45-47: Remove title-based agent inference from the label logic
around titleWords and agent in TerminalPane. Use the typed registered-agent
descriptor as the sole identity source, and when it is unavailable, fall back to
the localized tab number without appending an agent name.
In `@cmux-tui/frontends/web/src/hooks/useRenderTerminal.ts`:
- Line 442: Update the detached-event branch in useRenderTerminal to fail
closed: immediately clear the rendered terminal model and reset controllerRef
before returning. Ensure stale terminal state is no longer visible or able to
receive input when the authoritative attachment reports detached.
In `@cmux-tui/frontends/web/src/lib/keyEncoding.ts`:
- Around line 1-8: Update TerminalKeyEvent and the key-encoding logic to expose
and check getModifierState("AltGraph") before handling the ctrlKey branch. When
AltGraph is active, encode printable keys as text rather than as Ctrl+Alt
control sequences, while preserving existing behavior for other modifier
combinations.
In `@cmux-tui/frontends/web/src/styles.css`:
- Line 48: Update the CSS declarations in styles.css, including the font-family
declaration and the additionally affected locations, to remove quotes around
SFMono-Regular and replace currentColor with the configured lowercase
currentcolor spelling. Preserve all other styling and font fallback values.
In `@cmux-tui/spec/cli.md`:
- Line 97: Escape the literal pipe characters in the specified Markdown table
cells: update cmux-tui/spec/cli.md lines 97-97 and cmux-tui/spec/commands.md
lines 1945-1945 from coarse|deltas to coarse\|deltas, and
cmux-tui/spec/commands.md lines 2003-2003 from bytes|render to bytes\|render.
In `@cmux-tui/spec/frontends.md`:
- Around line 11-19: Update the browser or remote-capable frontend WebSocket
flow to document tokenless pairing: when no listener token is configured, send
{"pair":{"request":true}} as the first frame, complete the approval and
credential exchange, and only then send identify or other protocol requests.
Keep the existing auth preamble flow for listeners configured with a token.
In `@cmux-tui/spec/render.md`:
- Around line 185-186: Renumber the ordered open-question items in the section
containing the wide-grapheme and styled-cell entries so the visible list starts
at 1 and 2, preserving their existing order and content.
---
Outside diff comments:
In `@cmux-tui/crates/cmux-tui-core/src/mux.rs`:
- Around line 953-989: Serialize each workspace, pane, and screen mutation
together with its corresponding TreeDelta publication using a single ordered
mutation/event owner, including the paths around workspace creation and the
listed close/open operations. For close paths, snapshot the entities and collect
surfaces to kill while holding that owner, remove the entities and emit their
deltas before releasing it, then perform only the collected surface kills
afterward. Ensure concurrent commands cannot reorder mutation events or allow
newly added tabs/panes to survive a close delta.
In `@cmux-tui/crates/cmux-tui-core/src/server.rs`:
- Around line 555-604: Update WebSocketSink and run_websocket_writer to use a
bounded outbound queue with an explicit overflow/coalescing policy for terminal
frames, preventing unbounded memory growth. Apply finite write deadlines before
socket sends, and add cancellation that can interrupt both queued-message
draining and blocked writes so writer shutdown and join complete promptly.
Preserve delivery of already-queued responses where possible while ensuring
disconnect cannot wait indefinitely.
In `@cmux-tui/frontends/web/src/components/TerminalPane.tsx`:
- Around line 121-127: Update reportError in TerminalPane to stop storing
error.message for rendering; use the established localized, product-level
recovery message instead, while routing any sanitized diagnostic details through
internal telemetry if that mechanism already exists. Keep the existing client
and surface association unchanged so only the displayed error content changes.
In `@cmux-tui/spec/events.md`:
- Around line 716-734: Update the colors-changed payload schema to document
versioned shapes: protocol v6 omits surface, while protocol v7 requires surface:
Id. Keep the existing color and cursor fields unchanged, and make the v7
requirement explicit so clients can route events across multiple attachments.
🪄 Autofix (Beta)
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
Run ID: 3db70893-0557-4358-8d49-6b1145003288
⛔ Files ignored due to path filters (3)
cmux-tui/Cargo.lockis excluded by!**/*.lockcmux-tui/frontends/swift/CmuxLite/CmuxLite.xcodeproj/project.xcworkspace/contents.xcworkspacedatais excluded by!**/*.xcworkspace/contents.xcworkspacedatacmux-tui/frontends/web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (224)
.github/workflows/cmux-tui.ymlcmux-tui/Cargo.tomlcmux-tui/bindings/conformance/fixtures.jsoncmux-tui/bindings/go/client.gocmux-tui/bindings/go/cmd/e2e/main.gocmux-tui/bindings/java/src/com/cmux/CmuxClient.javacmux-tui/bindings/java/src/com/cmux/E2e.javacmux-tui/bindings/python/e2e.pycmux-tui/bindings/rust/examples/e2e.rscmux-tui/bindings/rust/src/lib.rscmux-tui/bindings/typescript/README.mdcmux-tui/bindings/typescript/e2e/e2e.tscmux-tui/bindings/typescript/src/browser.tscmux-tui/bindings/typescript/src/client.tscmux-tui/bindings/typescript/src/index.tscmux-tui/bindings/typescript/src/protocol/commands.tscmux-tui/bindings/typescript/src/protocol/events.tscmux-tui/bindings/typescript/src/protocol/index.tscmux-tui/bindings/typescript/src/protocol/render.tscmux-tui/bindings/typescript/src/types.tscmux-tui/bindings/typescript/src/websocket-transport.tscmux-tui/bindings/typescript/test/client.test.tscmux-tui/bindings/typescript/test/protocol-types.tscmux-tui/bindings/typescript/test/websocket-transport.test.tscmux-tui/crates/cmux-tui-cdp/src/client.rscmux-tui/crates/cmux-tui-core/Cargo.tomlcmux-tui/crates/cmux-tui-core/src/lib.rscmux-tui/crates/cmux-tui-core/src/mux.rscmux-tui/crates/cmux-tui-core/src/pairing.rscmux-tui/crates/cmux-tui-core/src/platform.rscmux-tui/crates/cmux-tui-core/src/server.rscmux-tui/crates/cmux-tui-core/src/surface.rscmux-tui/crates/cmux-tui-core/tests/pty.rscmux-tui/crates/cmux-tui-core/tests/websocket_transport.rscmux-tui/crates/cmux-tui/src/app.rscmux-tui/crates/cmux-tui/src/cli.rscmux-tui/crates/cmux-tui/src/config.rscmux-tui/crates/cmux-tui/src/main.rscmux-tui/crates/cmux-tui/src/session/mod.rscmux-tui/crates/cmux-tui/src/session/remote.rscmux-tui/crates/cmux-tui/src/ui/mod.rscmux-tui/crates/cmux-tui/src/ui/overlay.rscmux-tui/crates/cmux-tui/src/ui/pane.rscmux-tui/crates/cmux-tui/src/ui/sidebar.rscmux-tui/crates/cmux-tui/src/ui/terminal_grid.rscmux-tui/crates/cmux-tui/tests/cli.rscmux-tui/crates/ghostty-vt/src/lib.rscmux-tui/crates/ghostty-vt/src/render.rscmux-tui/crates/ghostty-vt/src/terminal.rscmux-tui/crates/ghostty-vt/tests/render_v7.rscmux-tui/crates/ghostty-vt/tests/terminal.rscmux-tui/docs/configuration.mdcmux-tui/frontends/swift/CmuxLite/.gitignorecmux-tui/frontends/swift/CmuxLite/CmuxLite.xcodeproj/project.pbxprojcmux-tui/frontends/swift/CmuxLite/CmuxLite.xcodeproj/xcshareddata/xcschemes/CmuxLite.xcschemecmux-tui/frontends/swift/CmuxLite/Package.swiftcmux-tui/frontends/swift/CmuxLite/README.ja.mdcmux-tui/frontends/swift/CmuxLite/README.mdcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxGhosttyConfigurationResolver.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxHoverButton.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxLiteApp.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxLiteWindowController.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxPalette.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxPaneViewController.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxPendingRatio.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxRenderColor.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxRenderFontMetrics.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxRenderView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxSplitDividerView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxSplitView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxStateDump.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxTabButton.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxTerminalGridContainerView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxTerminalHostViewController.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxWindowMutation.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/CmuxWorkspaceRowView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/NSEvent+CmuxShortcutInput.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/NSEvent+CmuxTerminalKeyEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteApp/Resources/Localizable.xcstringscmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxAttachEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxCommandRequest.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxConnectionConfiguration.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxConnectionEndpoint.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxEmptyResponse.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxFrontendEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxFrontendSession.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxFrontendStartup.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxGhosttyViewConfiguration.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxGhosttyViewConfigurationParser.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxIdentifyResponse.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxInboundHeader.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxLayout.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxLayoutRect.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxLocalSelection.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPane.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneAttachment.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneDirection.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneGeometry.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneLayoutView+Actions.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneLayoutView.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxPaneSnapshot.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxProtocolClient.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxProtocolClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxProtocolError.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRatioNudge.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxReadScrollbackResponse.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderAttributes.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderCursor.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderCursorStyle.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderDeltaEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderModel.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderRow.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderRun.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderStateEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderStyle.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxRenderUnderline.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxResizeAction.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxResizePolicy.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxResponseEnvelope.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScreen.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScreenSnapshot.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScrollbackDirection.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScrollbackReconciliation.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScrollbackRequest.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxScrollbackWindow.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxShortcutAction.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxShortcutInput.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxShortcutKey.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxShortcutModifiers.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxShortcutTable.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSplitDirection.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSplitRatio.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSplitTarget.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSurface.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSurfaceResponse.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxSurfaceSize.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTabSnapshot.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTerminalGridGeometry.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTerminalKeyAction.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTerminalKeyEvent.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTerminalMeasurement.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxTransport.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxWorkspace.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxWorkspaceSnapshot.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/CmuxWorkspaceTree.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/ConfiguredCmuxProtocolClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/FallbackCmuxProtocolClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/FallbackWebSocketTransport.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/NetworkCmuxProtocolClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/NetworkWebSocketTransport.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/Resources/Localizable.xcstringscmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/URLSessionCmuxProtocolClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/URLSessionWebSocketTransport.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteCore/UnixSocketTransport.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteSmoke/CmuxLiteSmoke.swiftcmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteSmoke/Resources/Localizable.xcstringscmux-tui/frontends/swift/CmuxLite/Sources/CmuxLiteSmoke/SmokeRunner.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxAttachEventTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxConnectionConfigurationTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxFrontendSessionTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxGhosttyViewConfigurationTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxPaneLayoutLogicTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxRenderModelTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxRenderStyleTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxResizePolicyTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxScrollbackWindowTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxShortcutTableTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxTerminalGridGeometryTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxTerminalKeyEventTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxWorkspaceTreeTests.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/ScriptedClientFactory.swiftcmux-tui/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/ScriptedTransport.swiftcmux-tui/frontends/swift/CmuxLite/project.ymlcmux-tui/frontends/web/README.ja.mdcmux-tui/frontends/web/README.mdcmux-tui/frontends/web/package.jsoncmux-tui/frontends/web/src/App.tsxcmux-tui/frontends/web/src/components/ByteTerminal.tsxcmux-tui/frontends/web/src/components/ConnectScreen.tsxcmux-tui/frontends/web/src/components/ExtraKeysBar.tsxcmux-tui/frontends/web/src/components/RenderTerminal.tsxcmux-tui/frontends/web/src/components/Sidebar.tsxcmux-tui/frontends/web/src/components/StatusBar.tsxcmux-tui/frontends/web/src/components/TerminalFrame.tsxcmux-tui/frontends/web/src/components/TerminalPane.tsxcmux-tui/frontends/web/src/hooks/useAttachedTerminal.tscmux-tui/frontends/web/src/hooks/useCmuxClient.tscmux-tui/frontends/web/src/hooks/useRenderTerminal.tscmux-tui/frontends/web/src/i18n.tscmux-tui/frontends/web/src/lib/canvasTheme.tscmux-tui/frontends/web/src/lib/fit.tscmux-tui/frontends/web/src/lib/frameBatch.tscmux-tui/frontends/web/src/lib/keyEncoding.tscmux-tui/frontends/web/src/lib/renderModel.tscmux-tui/frontends/web/src/lib/renderStyles.tscmux-tui/frontends/web/src/lib/scrollback.tscmux-tui/frontends/web/src/lib/terminalSelection.tscmux-tui/frontends/web/src/lib/tree.tscmux-tui/frontends/web/src/lib/webglRenderer.tscmux-tui/frontends/web/src/styles.csscmux-tui/frontends/web/test/canvas-theme.test.tscmux-tui/frontends/web/test/connect-screen.test.tsxcmux-tui/frontends/web/test/fit.test.tscmux-tui/frontends/web/test/frame-batch.test.tscmux-tui/frontends/web/test/key-encoding.test.tscmux-tui/frontends/web/test/render-model.test.tscmux-tui/frontends/web/test/render-styles.test.tscmux-tui/frontends/web/test/render-terminal.test.tsxcmux-tui/frontends/web/test/scrollback.test.tscmux-tui/frontends/web/test/terminal-pane-split-drag.test.tsxcmux-tui/frontends/web/test/terminal-selection.test.tscmux-tui/frontends/web/test/tui-chrome.test.tsxcmux-tui/frontends/web/test/webgl-renderer.test.tscmux-tui/scripts/smoke-tui.pycmux-tui/spec/README.mdcmux-tui/spec/bindings.mdcmux-tui/spec/cli.mdcmux-tui/spec/commands.mdcmux-tui/spec/events.mdcmux-tui/spec/frontends.mdcmux-tui/spec/render.mdcmux-tui/spec/transports.mdscripts/check-package-resolved-policy.pyscripts/install-zig-ci.sh
Comment on lines
+355
to
+357
| const legacyBytes = options.bytes instanceof Uint8Array ? encodeBase64(options.bytes) : options.bytes; | ||
| const bytes = "base64" in options ? options.base64 : legacyBytes; | ||
| return this.request("send", { surface, text: options.text, bytes, paste: options.paste }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve legacy bytes when base64 is explicitly undefined.
"base64" in options selects undefined, so { bytes, base64: undefined } silently sends neither value after dropUndefined().
Proposed fix
- const bytes = "base64" in options ? options.base64 : legacyBytes;
+ const bytes = options.base64 !== undefined ? options.base64 : legacyBytes;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const legacyBytes = options.bytes instanceof Uint8Array ? encodeBase64(options.bytes) : options.bytes; | |
| const bytes = "base64" in options ? options.base64 : legacyBytes; | |
| return this.request("send", { surface, text: options.text, bytes, paste: options.paste }); | |
| const legacyBytes = options.bytes instanceof Uint8Array ? encodeBase64(options.bytes) : options.bytes; | |
| const bytes = options.base64 !== undefined ? options.base64 : legacyBytes; | |
| return this.request("send", { surface, text: options.text, bytes, paste: options.paste }); |
🤖 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/bindings/typescript/src/client.ts` around lines 355 - 357, Update
the bytes selection in the send request flow to use options.base64 only when it
is defined; otherwise retain the computed legacyBytes value. Preserve explicit
base64 values while ensuring an options object with base64: undefined still
sends the legacy bytes.
Comment on lines
+543
to
+553
| private matchesAttachEvent(event: UnknownEvent, surface: Id, mode: "bytes" | "render"): boolean { | ||
| // colors-changed is scoped by its attach connection and intentionally has | ||
| // no surface field in protocol v6. | ||
| if (event.event === "colors-changed") return true; | ||
| // no surface field in protocol v6. Protocol v7 includes the surface id. | ||
| if (event.event === "colors-changed") { | ||
| return mode === "bytes" && (!("surface" in event) || event.surface === surface); | ||
| } | ||
| if (!("surface" in event) || event.surface !== surface) return false; | ||
| return this.attachOnlyEvent(event.event) || event.event === "scroll-changed"; | ||
| if (event.event === "detached" || event.event === "scroll-changed") return true; | ||
| return mode === "render" | ||
| ? event.event === "render-state" || event.event === "render-delta" | ||
| : event.event === "vt-state" || event.event === "output" || event.event === "resized"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not discard future render events on shared transports.
RenderAttachEvent includes UnknownEvent, but this filter rejects every unrecognized event even when its surface matches. Dedicated and shared transports therefore expose different APIs. Accept unknown surface-scoped events after excluding known incompatible byte/subscribe events, or remove UnknownEvent from the promised return type and add coverage for both transport modes.
🤖 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/bindings/typescript/src/client.ts` around lines 543 - 553, The
matchesAttachEvent filter currently rejects unknown surface-scoped events,
causing shared and dedicated transports to differ. Update matchesAttachEvent to
accept unrecognized events whose surface matches, while still excluding known
events incompatible with the requested bytes or render mode; alternatively
remove UnknownEvent from the promised RenderAttachEvent return type and add
coverage for both transport modes.
Comment on lines
+187
to
+193
| if (message.paired && typeof message.paired === "object") { | ||
| const credential = (message.paired as Record<string, unknown>).credential; | ||
| if (typeof credential === "string") { | ||
| this.authenticated = true; | ||
| this.onPairingCredential?.(credential); | ||
| this.flushPending(); | ||
| return; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let consumer callbacks interrupt transport progress.
A reentrant send() from onPairingCredential overtakes already queued requests, while a thrown callback leaves them stranded. Similarly, a throwing rejection callback prevents close handlers from running. Complete internal queue/close work regardless of callback behavior.
Proposed fix
if (typeof credential === "string") {
this.authenticated = true;
- this.onPairingCredential?.(credential);
this.flushPending();
+ try {
+ this.onPairingCredential?.(credential);
+ } catch (error) {
+ this.fail(this.eventError(error));
+ }
return;
}- if (event?.code === 1008 && event.reason === "authentication failed") {
- this.onAuthenticationRejected?.();
+ try {
+ if (event?.code === 1008 && event.reason === "authentication failed") {
+ this.onAuthenticationRejected?.();
+ }
+ } catch (error) {
+ this.fail(this.eventError(error));
+ } finally {
+ for (const handler of this.closeHandlers) handler();
}
- for (const handler of this.closeHandlers) handler();Also applies to: 218-225
🤖 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/bindings/typescript/src/websocket-transport.ts` around lines 187 -
193, Update the pairing handling around the credential branch in the websocket
transport so internal state and pending-request flushing complete independently
of onPairingCredential: preserve queue order when the callback reentrantly calls
send(), and ensure a thrown callback does not strand queued requests. Apply the
same protection to the rejection callback path around the close handling so
close handlers always run even if the consumer callback throws.
Comment on lines
+1037
to
+1053
| let output = wait_for( | ||
| || { | ||
| while let Some(value) = read_json_line(&mut byte_reader) { | ||
| if value["event"] == "output" { | ||
| let bytes = base64::engine::general_purpose::STANDARD | ||
| .decode(value["data"].as_str().unwrap()) | ||
| .unwrap(); | ||
| if bytes.windows(b"render-multi".len()).any(|part| part == b"render-multi") { | ||
| return Some(bytes); | ||
| } | ||
| } | ||
| } | ||
| None | ||
| }, | ||
| Duration::from_secs(10), | ||
| ); | ||
| assert!(output.is_some(), "byte attachment stopped while render attachment was active"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Accumulate byte output across frames.
PTY output may split render-multi across multiple output events. The predicate discards prior chunks, making correct fanout intermittently fail; append decoded bytes to a buffer before searching.
🤖 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-tui-core/tests/pty.rs` around lines 1037 - 1053, Update
the output predicate in the PTY test to retain decoded bytes from every “output”
event in an accumulated buffer, then search the combined buffer for
“render-multi” instead of checking each chunk independently. Keep consuming
events until the marker is found or the existing timeout expires.
Comment on lines
+1069
to
+1096
| let raced_surface = surface.clone(); | ||
| let raced = std::thread::spawn(move || raced_surface.write_bytes(b"race-v7\n").unwrap()); | ||
| writeln!( | ||
| writer, | ||
| r#"{{"id":1,"cmd":"attach-surface","surface":{},"mode":"render"}}"#, | ||
| surface.id | ||
| ) | ||
| .unwrap(); | ||
| raced.join().unwrap(); | ||
|
|
||
| let deadline = Instant::now() + Duration::from_secs(10); | ||
| let mut marker_events = 0; | ||
| let mut saw_response = false; | ||
| while Instant::now() < deadline && (!saw_response || marker_events == 0) { | ||
| if let Some(value) = read_json_line(&mut reader) { | ||
| if value["id"] == 1 { | ||
| assert_eq!(value["ok"], true); | ||
| saw_response = true; | ||
| } | ||
| if matches!(value["event"].as_str(), Some("render-state" | "render-delta")) | ||
| && rendered_rows_text(&value).contains("race-v7") | ||
| { | ||
| marker_events += 1; | ||
| } | ||
| } | ||
| } | ||
| assert!(saw_response, "render attach response was not delivered"); | ||
| assert_eq!(marker_events, 1, "raced output was missing or duplicated across snapshot/delta"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make the snapshot/write race deterministic and drain past the response.
The spawned write may finish before attachment begins. The loop also exits after one marker and the response, so a queued duplicate can remain unread. Synchronize the write at the snapshot/subscription boundary and read through a known post-write sequence barrier before asserting exactly one marker event.
🤖 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-tui-core/tests/pty.rs` around lines 1069 - 1096, Update
the PTY attach race test around the spawned write and response-reading loop to
coordinate the writer with the snapshot/subscription boundary, ensuring the
write occurs after attachment is ready rather than merely racing by scheduling.
After issuing the attach request, drain events through a known post-write
sequence barrier instead of stopping once the response and first marker arrive,
then assert exactly one matching marker event after the barrier.
Comment on lines
+18
to
+24
| func fallsBackToFirstLivePTYWhenActiveTabIsBrowser() throws { | ||
| let json = Data( | ||
| #"{"workspaces":[{"id":4,"name":"one","active":true,"screens":[{"id":3,"name":null,"active":true,"active_pane":2,"panes":[{"id":2,"active_tab":0,"tabs":[{"surface":9,"kind":"browser","name":null,"title":"web","size":null,"dead":false},{"surface":12,"kind":"pty","name":null,"title":"shell","size":null,"dead":false}]}]}]}]}"#.utf8 | ||
| ) | ||
| let tree = try JSONDecoder().decode(CmuxWorkspaceTree.self, from: json) | ||
|
|
||
| #expect(tree.selectedSurface() == 12) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not codify fallback to a different tab when the active tab is unsupported.
The server identifies surface 9 as active, but this expectation silently selects surface 12. That can display and route input to a tab the user did not select. Represent the unsupported browser selection or return no terminal surface instead of guessing.
As per path instructions, correctness-critical surface selection must use the authoritative source and fail closed when it is unavailable.
🤖 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/frontends/swift/CmuxLite/Tests/CmuxLiteCoreTests/CmuxWorkspaceTreeTests.swift`
around lines 18 - 24, Update selectedSurface() and its test
fallsBackToFirstLivePTYWhenActiveTabIsBrowser so the active tab remains
authoritative: when the selected tab is an unsupported browser, represent that
selection or return no terminal surface instead of falling back to surface 12.
Preserve selection of the active terminal tab and fail closed when no usable
authoritative terminal surface exists.
Source: Path instructions
| @@ -42,7 +45,9 @@ | |||
| --ansi-bright-white: #ffffff; | |||
| color: var(--terminal-foreground); | |||
| background: var(--terminal-background); | |||
| font-family: "SFMono-Regular", Consolas, "Liberation Mono", monospace; | |||
| font-family: Menlo, "SFMono-Regular", Consolas, "Liberation Mono", monospace; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve the new Stylelint errors.
Remove the quotes around SFMono-Regular and use the configured lowercase currentcolor spelling.
Proposed fix
- font-family: Menlo, "SFMono-Regular", Consolas, "Liberation Mono", monospace;
+ font-family: Menlo, SFMono-Regular, Consolas, "Liberation Mono", monospace;
- color: currentColor;
+ color: currentcolor;
- background: currentColor;
+ background: currentcolor;Also applies to: 122-124, 134-135, 140-140, 161-161, 165-165
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 48-48: Expected no quotes around "SFMono-Regular" (font-family-name-quotes)
(font-family-name-quotes)
🤖 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/frontends/web/src/styles.css` at line 48, Update the CSS
declarations in styles.css, including the font-family declaration and the
additionally affected locations, to remove quotes around SFMono-Regular and
replace currentColor with the configured lowercase currentcolor spelling.
Preserve all other styling and font fallback values.
Source: Linters/SAST tools
| @@ -93,8 +94,8 @@ The generated CLI requires one of `--index` or `--delta` for `select-tab`, `sele | |||
| | `move-tab` | implemented | `--surface <id> --pane <id> --index <n>` | none | none | | |||
| | `move-workspace` | implemented | `--workspace <id> --index <n>` | none | none | | |||
| | `scroll-surface` | implemented | `--surface <id> --delta <n>` | none | none | | |||
| | `subscribe` | implemented | none | none in v5 | event JSON lines | | |||
| | `attach-surface` | implemented | `--surface <id>` | none | event JSON lines | | |||
| | `subscribe` | implemented; tree deltas protocol 7 | none | `--tree-events coarse|deltas` | event JSON lines | | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape literal pipes in CLI Markdown table cells.
Unescaped | characters split these cells into extra columns and break table rendering:
cmux-tui/spec/cli.md#L97-L97: changecoarse|deltastocoarse\|deltas.cmux-tui/spec/commands.md#L1945-L1945: changecoarse|deltastocoarse\|deltas.cmux-tui/spec/commands.md#L2003-L2003: changebytes|rendertobytes\|render.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 97-97: Table column count
Expected: 5; Actual: 6; Too many cells, extra data will be missing
(MD056, table-column-count)
📍 Affects 2 files
cmux-tui/spec/cli.md#L97-L97(this comment)cmux-tui/spec/commands.md#L1945-L1945cmux-tui/spec/commands.md#L2003-L2003
🤖 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/spec/cli.md` at line 97, Escape the literal pipe characters in the
specified Markdown table cells: update cmux-tui/spec/cli.md lines 97-97 and
cmux-tui/spec/commands.md lines 1945-1945 from coarse|deltas to coarse\|deltas,
and cmux-tui/spec/commands.md lines 2003-2003 from bytes|render to
bytes\|render.
Source: Linters/SAST tools
Comment on lines
+11
to
+19
| For a browser or remote-capable frontend, connect to the opt-in WebSocket listener. Send one complete JSON request per text frame and treat every received text frame as one complete response or event. Do not add newline framing. The TypeScript SDK exposes `WebSocketTransport` for browsers and compatible Node WebSocket implementations. | ||
|
|
||
| If the WebSocket listener has a token, its first frame must be the transport preamble below. It is not a command and the server sends no acknowledgement: | ||
| If the WebSocket listener has a token, its first frame must be this transport preamble. It is not a command and has no acknowledgement: | ||
|
|
||
| ```json | ||
| {"auth":{"token":"replace-with-a-secret"}} | ||
| ``` | ||
|
|
||
| Only after that preamble should the client send protocol requests. See [`transports.md`](transports.md#authentication-preamble) for rejection and bind rules. | ||
| Only then send protocol requests. See [`transports.md`](transports.md#authentication-preamble) for rejection and bind rules. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Document the tokenless WebSocket pairing path.
This canonical flow only explains the auth preamble when a token exists. Per cmux-tui/spec/transports.md Lines 112-132, tokenless clients must send {"pair":{"request":true}} as their first frame and complete approval/credential exchange before sending identify; otherwise the documented frontend flow is rejected.
🤖 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/spec/frontends.md` around lines 11 - 19, Update the browser or
remote-capable frontend WebSocket flow to document tokenless pairing: when no
listener token is configured, send {"pair":{"request":true}} as the first frame,
complete the approval and credential exchange, and only then send identify or
other protocol requests. Keep the existing auth preamble flow for listeners
configured with a token.
Comment on lines
+185
to
+186
| 3. The current wrapper exposes wide grapheme heads followed by empty cells but no explicit spacer discriminator. Confirm that the lower-level cell metadata can produce logical `text` plus an authoritative `width_hint` without confusing wide spacers with blank cells. | ||
| 4. The wrapper exposes styled cells only for the viewport; scrollback count and plain text are available, but non-mutating styled random access is not. `read-scrollback` requires a wrapper/engine API that does not move the shared viewport or consume its damage flags. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Renumber the open-question list.
This section starts its visible ordered list at 3. and 4., which triggers MD029 and implies missing items. Use 1./2. or bullets unless preserving the numbering is intentional.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 185-185: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 186-186: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
🤖 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/spec/render.md` around lines 185 - 186, Renumber the ordered
open-question items in the section containing the wide-grapheme and styled-cell
entries so the visible list starts at 1 and 2, preserving their existing order
and content.
Source: Linters/SAST tools
Comment on lines
+540
to
+605
| /// Record one viewer's available grid and resize the shared surface to | ||
| /// the smallest rows and columns reported by all current viewers. | ||
| pub fn resize_surface_for_client( | ||
| &self, | ||
| id: SurfaceId, | ||
| client: u64, | ||
| cols: u16, | ||
| rows: u16, | ||
| ) -> anyhow::Result<bool> { | ||
| let requested = (cols.max(1), rows.max(1)); | ||
| let effective = { | ||
| let mut sizes = self.client_surface_sizes.lock().unwrap(); | ||
| let viewers = sizes.entry(id).or_default(); | ||
| viewers.insert(client, requested); | ||
| viewers | ||
| .values() | ||
| .copied() | ||
| .fold(requested, |smallest, size| (smallest.0.min(size.0), smallest.1.min(size.1))) | ||
| }; | ||
| self.record_client_size(effective.0, effective.1); | ||
| match self.resize_surface(id, effective.0, effective.1) { | ||
| Ok(changed) => Ok(changed), | ||
| Err(error) => { | ||
| let mut sizes = self.client_surface_sizes.lock().unwrap(); | ||
| if let Some(viewers) = sizes.get_mut(&id) { | ||
| viewers.remove(&client); | ||
| if viewers.is_empty() { | ||
| sizes.remove(&id); | ||
| } | ||
| } | ||
| Err(error) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn remove_surface_size_client(&self, id: SurfaceId, client: u64) { | ||
| let effective = { | ||
| let mut sizes = self.client_surface_sizes.lock().unwrap(); | ||
| let Some(viewers) = sizes.get_mut(&id) else { return }; | ||
| viewers.remove(&client); | ||
| let effective = viewers | ||
| .values() | ||
| .copied() | ||
| .reduce(|smallest, size| (smallest.0.min(size.0), smallest.1.min(size.1))); | ||
| if viewers.is_empty() { | ||
| sizes.remove(&id); | ||
| } | ||
| effective | ||
| }; | ||
| if let Some((cols, rows)) = effective { | ||
| let _ = self.resize_surface(id, cols, rows); | ||
| } | ||
| } | ||
|
|
||
| pub fn remove_size_client(&self, client: u64) { | ||
| let surface_ids = self | ||
| .client_surface_sizes | ||
| .lock() | ||
| .unwrap() | ||
| .iter() | ||
| .filter_map(|(surface, viewers)| viewers.contains_key(&client).then_some(*surface)) | ||
| .collect::<Vec<_>>(); | ||
| for surface in surface_ids { | ||
| self.remove_surface_size_client(surface, client); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Serialize size reconciliation with the size-map update.
The size mutex is released before resizing. A concurrent resize can update the map to 60 columns, then an older call can resume and resize to 120, leaving the map and PTY inconsistent. Hold one reconciliation owner through calculation and application, or version/recheck before applying.
🤖 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-tui-core/src/mux.rs` around lines 540 - 605, Serialize
size reconciliation in resize_surface_for_client, remove_surface_size_client,
and remove_size_client so map updates and the corresponding resize_surface
application cannot interleave with stale calculations. Hold the
client_surface_sizes reconciliation lock through effective-size calculation and
resize application, or recheck/version the map immediately before applying,
ensuring the PTY size always matches the latest recorded viewers.
Comment on lines
+142
to
+162
| pub(crate) fn respond(&self, id: u64, approve: bool) -> bool { | ||
| let now = Instant::now(); | ||
| let mut state = self.state.lock().unwrap(); | ||
| Self::prune(&mut state, now); | ||
| let Some(request) = state.pending.remove(&id) else { return false }; | ||
| let decision = if approve { | ||
| let Ok(value) = random_credential() else { | ||
| let _ = request.response.send(PairingDecision::Denied); | ||
| return false; | ||
| }; | ||
| state | ||
| .credentials | ||
| .push_back(Credential { value: value.clone(), expires_at: now + CREDENTIAL_TTL }); | ||
| while state.credentials.len() > MAX_CREDENTIALS { | ||
| state.credentials.pop_front(); | ||
| } | ||
| PairingDecision::Approved { credential: value } | ||
| } else { | ||
| PairingDecision::Denied | ||
| }; | ||
| request.response.send(decision).is_ok() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not equate response delivery with request validity.
The request is removed before credential generation and delivery, but false makes Mux report it as unknown and skip PairingResolved. A disconnected receiver also leaves an unreachable credential stored. Return a structured outcome distinguishing not-found from delivery failure, and always resolve requests that were found.
🤖 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-tui-core/src/pairing.rs` around lines 142 - 162, The
respond method currently conflates missing requests with response-channel
delivery failures and stores credentials even when delivery fails. Update
PairingManager::respond and its callers to return a structured outcome
distinguishing not-found from delivery failure, ensure every found request emits
PairingResolved regardless of response delivery status, and only retain a
generated credential when the approved decision can be delivered successfully.
Comment on lines
939
to
946
| if thread_connections.lock().unwrap().len() >= MAX_WEBSOCKET_CONNECTIONS { | ||
| let _ = stream.shutdown(Shutdown::Both); | ||
| continue; | ||
| } | ||
| let id = next_connection.fetch_add(1, Ordering::Relaxed); | ||
| if let Ok(tracked) = stream.try_clone() { | ||
| thread_connections.lock().unwrap().insert(id, tracked); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject connections that cannot be tracked.
When try_clone() fails, the connection is still spawned but is absent from the connection cap and shutdown registry.
Proposed fix
- if let Ok(tracked) = stream.try_clone() {
- thread_connections.lock().unwrap().insert(id, tracked);
- }
+ let Ok(tracked) = stream.try_clone() else {
+ let _ = stream.shutdown(Shutdown::Both);
+ continue;
+ };
+ thread_connections.lock().unwrap().insert(id, tracked);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if thread_connections.lock().unwrap().len() >= MAX_WEBSOCKET_CONNECTIONS { | |
| let _ = stream.shutdown(Shutdown::Both); | |
| continue; | |
| } | |
| let id = next_connection.fetch_add(1, Ordering::Relaxed); | |
| if let Ok(tracked) = stream.try_clone() { | |
| thread_connections.lock().unwrap().insert(id, tracked); | |
| } | |
| if thread_connections.lock().unwrap().len() >= MAX_WEBSOCKET_CONNECTIONS { | |
| let _ = stream.shutdown(Shutdown::Both); | |
| continue; | |
| } | |
| let id = next_connection.fetch_add(1, Ordering::Relaxed); | |
| let Ok(tracked) = stream.try_clone() else { | |
| let _ = stream.shutdown(Shutdown::Both); | |
| continue; | |
| }; | |
| thread_connections.lock().unwrap().insert(id, tracked); |
🤖 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-tui-core/src/server.rs` around lines 939 - 946, The
connection-acceptance path must reject streams that cannot be registered in
thread_connections. Update the try_clone handling after
next_connection.fetch_add in the server loop to shut down the original stream
and continue when cloning fails; only spawn or proceed with connections after
successfully inserting the cloned stream into the tracking registry.
Comment on lines
+1398
to
+1444
| pub(crate) fn tree_entity_json( | ||
| state: &State, | ||
| notifications: &HashMap<SurfaceId, SurfaceNotification>, | ||
| kind: TreeDeltaKind, | ||
| id: u64, | ||
| ) -> Option<Value> { | ||
| let tree = workspaces_json(state, notifications); | ||
| let workspaces = tree.get("workspaces")?.as_array()?; | ||
| match kind { | ||
| TreeDeltaKind::WorkspaceAdded | ||
| | TreeDeltaKind::WorkspaceClosed | ||
| | TreeDeltaKind::WorkspaceRenamed => workspaces | ||
| .iter() | ||
| .find(|workspace| workspace.get("id").and_then(Value::as_u64) == Some(id)) | ||
| .cloned(), | ||
| TreeDeltaKind::ScreenAdded | TreeDeltaKind::ScreenClosed | TreeDeltaKind::ScreenRenamed => { | ||
| workspaces | ||
| .iter() | ||
| .flat_map(|workspace| { | ||
| workspace.get("screens").and_then(Value::as_array).into_iter().flatten() | ||
| }) | ||
| .find(|screen| screen.get("id").and_then(Value::as_u64) == Some(id)) | ||
| .cloned() | ||
| } | ||
| TreeDeltaKind::PaneAdded | TreeDeltaKind::PaneClosed => workspaces | ||
| .iter() | ||
| .flat_map(|workspace| { | ||
| workspace.get("screens").and_then(Value::as_array).into_iter().flatten() | ||
| }) | ||
| .flat_map(|screen| screen.get("panes").and_then(Value::as_array).into_iter().flatten()) | ||
| .find(|pane| pane.get("id").and_then(Value::as_u64) == Some(id)) | ||
| .cloned(), | ||
| TreeDeltaKind::TabAdded | TreeDeltaKind::TabClosed | TreeDeltaKind::TabRenamed => { | ||
| workspaces | ||
| .iter() | ||
| .flat_map(|workspace| { | ||
| workspace.get("screens").and_then(Value::as_array).into_iter().flatten() | ||
| }) | ||
| .flat_map(|screen| { | ||
| screen.get("panes").and_then(Value::as_array).into_iter().flatten() | ||
| }) | ||
| .flat_map(|pane| pane.get("tabs").and_then(Value::as_array).into_iter().flatten()) | ||
| .find(|tab| tab.get("surface").and_then(Value::as_u64) == Some(id)) | ||
| .cloned() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Serialize only the entity needed for each delta.
workspaces_json materializes the entire tree and then scans it for one entity. Each mutation is O(tree size), making N tab/workspace additions O(N²) and extending the state-lock duration around roughly 1,000 records.
Build the targeted workspace, screen, pane, or tab JSON directly from the authoritative state.
As per coding guidelines, avoid repeated full scans over scalable production collections and use indexes or single-pass plans.
🤖 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-tui-core/src/server.rs` around lines 1398 - 1444, Update
tree_entity_json to construct only the requested workspace, screen, pane, or tab
JSON directly from authoritative State data instead of calling workspaces_json
and scanning the materialized tree. Use existing state indexes or single-pass
lookups keyed by the provided id, preserve the current Option<Value> behavior,
and keep each TreeDeltaKind mapped to its corresponding entity serializer.
Source: Coding guidelines
Comment on lines
+1924
to
+1944
| Command::ReadScrollback { surface, start, count } => { | ||
| let surface = get_surface(mux, surface)?; | ||
| require_pty(&surface)?; | ||
| let count = u16::try_from(count).map_err(|_| anyhow::anyhow!("count out of range"))?; | ||
| let (start, total, rows) = surface.try_with_terminal(|term| { | ||
| let total = term.history_rows(); | ||
| let start = start.min(total); | ||
| term.styled_history_rows(start, count).map(|rows| (start, total, rows)) | ||
| })??; | ||
| let runs = rows_to_runs(&rows); | ||
| let rows = runs | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(row, runs)| { | ||
| json!({ | ||
| "row": row as u16, | ||
| "runs": runs.iter().map(styled_run_json).collect::<Vec<_>>(), | ||
| }) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| Ok(json!({ "rows": rows, "start": start, "total": total })) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Cap scrollback page size before materializing it.
Converting to u16 still permits 65,535 styled rows in one response, which can consume substantial memory and block serialization. Enforce a small protocol page limit and reject or clamp larger requests.
🤖 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-tui-core/src/server.rs` around lines 1924 - 1944, Limit
the requested page size in the Command::ReadScrollback handler before calling
styled_history_rows, using a small protocol-defined maximum; reject or clamp
counts above that limit while preserving the existing u16 validation and
response behavior.
| <small>{t("pairingExpires", { seconds: pairing.expiresIn })}</small> | ||
| </div> | ||
| )} | ||
| {error && <div className="inline-error" role="alert">{error || t("unknownError")}</div>} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not render raw transport or server errors.
Pairing failures can carry the upstream pairing_error.message directly into this alert. Map them to localized product-level errors and keep implementation details internal.
As per coding guidelines, “Do not expose raw upstream errors.” <coding_guidelines>
🤖 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/frontends/web/src/components/ConnectScreen.tsx` at line 67, Update
the error alert in ConnectScreen to avoid rendering the raw error value. Map
pairing and transport/server failures to localized product-level messages using
the existing translation function t, with unknown or unmapped failures falling
back to t("unknownError"), while preserving the alert’s role and display
behavior.
Source: Coding guidelines
Comment on lines
38
to
40
| <button className={`screen-chip${screen.active ? " active" : ""}`} onClick={onSelect} type="button"> | ||
| {number} | ||
| {screen.statusLabel ?? number} | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Constrain user-defined screen names in the status bar.
An arbitrarily long screen name can consume the nowrap flex row and hide subsequent screens, the new-screen action, and the session badge.
cmux-tui/frontends/web/src/components/StatusBar.tsx#L38-L40: expose the full label throughtitleor an accessible label.cmux-tui/frontends/web/src/lib/tree.ts#L63-L72: continue treatingstatusLabelas potentially unbounded user input.cmux-tui/frontends/web/src/styles.css#L178-L180: add a maximum width,overflow: hidden, andtext-overflow: ellipsisto screen chips.
📍 Affects 3 files
cmux-tui/frontends/web/src/components/StatusBar.tsx#L38-L40(this comment)cmux-tui/frontends/web/src/lib/tree.ts#L63-L72cmux-tui/frontends/web/src/styles.css#L178-L180
🤖 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/frontends/web/src/components/StatusBar.tsx` around lines 38 - 40,
Constrain unbounded user-defined screen labels without truncating their
accessible full value: in cmux-tui/frontends/web/src/components/StatusBar.tsx
lines 38-40, add a title or accessible label to the screen button; in
cmux-tui/frontends/web/src/lib/tree.ts lines 63-72, make no direct change and
continue treating statusLabel as unbounded input; in
cmux-tui/frontends/web/src/styles.css lines 178-180, add a maximum width, hidden
overflow, and ellipsis text overflow to the screen-chip styling.
Comment on lines
+45
to
+47
| const titleWords = tab.title.toLowerCase().split(/[^a-z0-9_-]+/); | ||
| const agent = ["claude", "codex", "opencode", "pi"].find((candidate) => titleWords.includes(candidate)); | ||
| const label = tab.name || `${index + 1}${agent ? ` ${agent}` : ""}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not infer the running agent from the tab title.
Titles are mutable and may contain claude, codex, etc. without identifying the actual agent, causing a false identity label. Use a typed registered-agent descriptor; when unavailable, fall back to the localized tab number without an agent claim.
As per path instructions, agent identity must use a reliable structured source and fail closed when that signal is missing.
🤖 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/frontends/web/src/components/TerminalPane.tsx` around lines 45 - 47,
Remove title-based agent inference from the label logic around titleWords and
agent in TerminalPane. Use the typed registered-agent descriptor as the sole
identity source, and when it is unavailable, fall back to the localized tab
number without appending an agent name.
Sources: Coding guidelines, Path instructions
| throw error; | ||
| } | ||
| if (cancelled) return; | ||
| if (event.event === "detached") return; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail closed when the authoritative attachment reports detached.
Returning here leaves the previous model visible and controllerRef able to send input after the attachment has ended. Clear both immediately so stale terminal state cannot remain interactive.
Proposed fix
- if (event.event === "detached") return;
+ if (event.event === "detached") {
+ controllerRef.current = null;
+ dispatch({ type: "reset", client, surface });
+ return;
+ }As per path instructions, typed lifecycle events are authoritative and missing liveness must fail closed rather than preserve stale UI state.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (event.event === "detached") return; | |
| if (event.event === "detached") { | |
| controllerRef.current = null; | |
| dispatch({ type: "reset", client, surface }); | |
| return; | |
| } |
🤖 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/frontends/web/src/hooks/useRenderTerminal.ts` at line 442, Update
the detached-event branch in useRenderTerminal to fail closed: immediately clear
the rendered terminal model and reset controllerRef before returning. Ensure
stale terminal state is no longer visible or able to receive input when the
authoritative attachment reports detached.
Sources: Coding guidelines, Path instructions
Comment on lines
+1
to
+8
| export interface TerminalKeyEvent { | ||
| key: string; | ||
| ctrlKey: boolean; | ||
| altKey: boolean; | ||
| shiftKey: boolean; | ||
| metaKey: boolean; | ||
| isComposing?: boolean; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== outline keyEncoding.ts ==\n'
ast-grep outline cmux-tui/frontends/web/src/lib/keyEncoding.ts --view expanded || true
printf '\n== search AltGraph / key encoding references ==\n'
rg -n --hidden -S 'AltGraph|getModifierState|encodeTerminalKey|TerminalKeyEvent' cmux-tui/frontends/web/src/lib cmux-tui/frontends/web/src/test cmux-tui/frontends/web/src || true
printf '\n== keyEncoding.ts contents ==\n'
cat -n cmux-tui/frontends/web/src/lib/keyEncoding.ts | sed -n '1,220p'Repository: manaflow-ai/cmux
Length of output: 5481
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' cmux-tui/frontends/web/src/lib/keyEncoding.ts | cat -nRepository: manaflow-ai/cmux
Length of output: 3352
Handle AltGraph before Ctrl+Alt encoding
On layouts where AltGraph surfaces as ctrlKey && altKey, printable keys such as @ are sent as control sequences instead of text. Treat getModifierState("AltGraph") as printable input before the ctrlKey branch.
🤖 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/frontends/web/src/lib/keyEncoding.ts` around lines 1 - 8, Update
TerminalKeyEvent and the key-encoding logic to expose and check
getModifierState("AltGraph") before handling the ctrlKey branch. When AltGraph
is active, encode printable keys as text rather than as Ctrl+Alt control
sequences, while preserving existing behavior for other modifier combinations.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cmux-tui.yml (1)
4-5: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore automatic CI triggers here.
.github/workflows/cmux-tui.yml:4-5is manual-only, socmux-tuibuilds/tests won’t run on PRs or pushes. Re-enablepull_request/pushtriggers, or keep a separate automatic gate before merge.🤖 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 @.github/workflows/cmux-tui.yml around lines 4 - 5, Update the workflow trigger configuration near workflow_dispatch to restore automatic CI execution for both pull_request and push events, while retaining manual workflow_dispatch support. Ensure cmux-tui builds and tests run on pull requests and pushes.
🤖 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.
Outside diff comments:
In @.github/workflows/cmux-tui.yml:
- Around line 4-5: Update the workflow trigger configuration near
workflow_dispatch to restore automatic CI execution for both pull_request and
push events, while retaining manual workflow_dispatch support. Ensure cmux-tui
builds and tests run on pull requests and pushes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2153767-ce86-4a66-a351-1cb2040d7f32
📒 Files selected for processing (1)
.github/workflows/cmux-tui.yml
2 of 4 tasks
This was referenced Jul 18, 2026
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
1 participant
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.
Stacked on #7924.
Protocol v7 makes the server's terminal state the product for rich clients instead of raw PTY bytes, closing the emulator-drift class (cursor position/style, colors, scrollback) by construction. attach-surface gains mode:"render": a render-state snapshot (styled maximal runs with resolved RGB and explicit underline variants, full cursor visuals, defaults, scrollback length) followed by damage-coalesced render-delta events from a per-surface frame producer that fans one immutable frame to the TUI and every attachment; byte-only servers skip frame construction entirely. read-scrollback pages styled history without touching the viewport. subscribe tree_events:"deltas" opts into 11 entity events (coarse default keeps v6 subscribers exact). colors-changed names its surface; send paste:true does bracketed paste iff mode 2004. Everything additive: absent v7 selectors give byte-identical v6 behavior.
The web frontend renders server truth against v7 servers (DOM runs, real cursor element, native text selection, bidirectional server-paged scrollback with growth keeping your reading position) and keeps xterm.js for v6 servers. The ghostty-vt wrapper gains shared render frames, wide-cell spacer discrimination, and non-mutating styled history reads, all within the existing upstream C API. New: frontends/swift/CmuxLite, a minimal Swift libghostty (GhosttyKit) frontend speaking byte mode, live-smoked against a running session.
Every layer judge-reviewed (spec design review first, then engine/server/SDK/web rounds); live-verified end to end against a headless v7 server: render-mode selection, typing to disk, DECSCUSR cursor style changes rendering live, default colors from the snapshot, scrollback paging through 500 lines of history with back-to-live. Known follow-up: headless servers don't yet apply the ghostty-config cursor default to the render cursor (DECSCUSR works; the config default only lands via the TUI path).
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds protocol v7 with server-rendered terminal attachments, server‑paged scrollback, opt‑in tree delta events, and a TUI‑approved WebSocket pairing flow. Also introduces the Swift
cmux-litefrontend and fixes shared sizing to preserve the smallest‑viewer size across lifecycle events.New Features
cmux-tui-core):attach-surfacewithmode:"render"streamsrender-stateand coalescedrender-delta;read-scrollbackpages styled history;subscribe({ tree_events: "deltas" }); all events includesurface;send({ paste: true })wraps bracketed paste when DEC 2004 is active.--ws-tokenbypass); dedicated WS writer + poll‑based reader andTCP_NODELAYremove the 100ms stall for sub‑ms delivery.xterm.js+@xterm/addon-webglfor v6 and auto‑enables v7.frontends/swift/CmuxLiterenders v7 rows and cursor, supports server‑paged scrollback with Back to live, tabs/splits with optimistic ratio drags, crisp 1:1 grids with letterboxing for foreign sizes, Ghostty‑parity fonts/theme, Unix‑socket default with--urlfor WebSocket, and a SIGUSR1 state dump.RenderState/RenderDelta),readScrollback,subscribe({ treeEvents: "deltas" }),send({ base64, paste }), and pairing callbacks; CLI addsread-scrollback; Go/Java/Rust accept v7 for default byte‑mode attach.CMUX_TEST_WS_LATENCY_BUDGET_MS(valgrind job sets 2000ms); valgrind‑robust WS reader; strict clippy gate satisfied; tests cover smallest‑size lifecycle preservation.Migration
protocol >= 7and useattach-surfacewithmode:"render"for server‑rendered frames; omitmodefor v6/v7 byte‑identical streams.authTokento the TS transport or start the server with--ws-tokento bypass interactive approval.cmux-liteand the TUI remote client require protocol 7; the web app supports v6 and opts into v7 when available.send({ text })or{ base64 }; use{ paste: true }for bracketed paste; subscribe to deltas withsubscribe({ treeEvents: "deltas" }).cmux-litedefaults to the Unix socket; use--urlfor WebSocket.Written for commit c2a0c2b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation