Repository navigation
remote-tmux: strip the screen/tmux ESC k window-title escape from mirror output - #7023
Conversation
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesRemote Tmux Screen Title Filter
Sequence Diagram(s)sequenceDiagram
participant tmux as Remote tmux
participant Mirror as RemoteTmuxSessionMirror
participant Filter as RemoteTmuxScreenTitleFilter
participant Surface as WindowMirror or TerminalPanel
tmux->>Mirror: routeOutput(paneId, raw data)
Mirror->>Filter: filter(raw data)
Filter-->>Mirror: cleaned data
Mirror->>Surface: route cleaned data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR fixes a terminal rendering bug where shell prompts running inside tmux (with
Confidence Score: 5/5Safe to merge — the filter is correctly applied to all output paths, state resets properly on disconnect, and the logic is fully covered by chunk-split fuzzing and edge-case tests. The change is narrow and well-contained: a new value-type byte filter applied in one call site, with no actor isolation issues (all mutations stay on the main actor), correct pruning alongside the existing cwdByPane cache, and an existing connection-state hook used to clear stale filter state before reseed. The state machine correctly handles consecutive ESCs, BEL-not-terminating, and every chunk-split boundary. No blocking primitives, no ambient globals, and no test seams in production source. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant tmux as Remote tmux (%output)
participant conn as RemoteTmuxControlConnection
participant mirror as RemoteTmuxSessionMirror
participant filter as RemoteTmuxScreenTitleFilter (per pane)
participant surface as TerminalSurface / WindowMirror
tmux->>conn: raw pty bytes (may contain ESC k title ESC \\)
conn->>mirror: onPaneOutput(paneId, data)
mirror->>filter: filter(data)
note over filter: fast-path: no ESC then return data<br/>otherwise: strip ESC k ESC\\ sequences<br/>stateful across chunk splits
filter-->>mirror: cleaned Data
mirror->>surface: routeOutput / processRemoteOutput(cleaned)
note over mirror: onConnectionStateChanged not connected<br/>titleFilters.removeAll()
note over mirror: rebuild() titleFilters.filter(livePanes)
%%{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 tmux as Remote tmux (%output)
participant conn as RemoteTmuxControlConnection
participant mirror as RemoteTmuxSessionMirror
participant filter as RemoteTmuxScreenTitleFilter (per pane)
participant surface as TerminalSurface / WindowMirror
tmux->>conn: raw pty bytes (may contain ESC k title ESC \\)
conn->>mirror: onPaneOutput(paneId, data)
mirror->>filter: filter(data)
note over filter: fast-path: no ESC then return data<br/>otherwise: strip ESC k ESC\\ sequences<br/>stateful across chunk splits
filter-->>mirror: cleaned Data
mirror->>surface: routeOutput / processRemoteOutput(cleaned)
note over mirror: onConnectionStateChanged not connected<br/>titleFilters.removeAll()
note over mirror: rebuild() titleFilters.filter(livePanes)
Reviews (7): Last reviewed commit: "Satisfy remote tmux filter package polic..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.xcodeproj/project.pbxproj`:
- Line 3096: RemoteTmuxScreenTitleFilter is standalone parsing/state-machine
logic that should not live in the app target root Sources. Move the
RemoteTmuxScreenTitleFilter.swift implementation into an existing remote/session
package target or a small new reusable module, then update the app target to
depend on that module instead of owning the type directly. Keep the type name
and any callers/imports consistent so the app still references
RemoteTmuxScreenTitleFilter from the new package boundary.
In `@cmuxTests/RemoteTmuxScreenTitleFilterTests.swift`:
- Around line 16-22: The test helper in RemoteTmuxScreenTitleFilterTests is
decoding filtered output with String(decoding:as:), which can hide invalid UTF-8
and mask byte-stream regressions. Update the run(_:) helpers to assert on the
raw Data returned from RemoteTmuxScreenTitleFilter.filter, or switch to a
throwing/optional UTF-8 conversion such as String(bytes:encoding:) so malformed
output fails instead of being silently replaced.
In `@Sources/RemoteTmuxScreenTitleFilter.swift`:
- Around line 36-77: The hot path in `RemoteTmuxScreenTitleFilter.filter(_:)` is
doing per-byte `Data.append`, which is too costly for `%output` chunks that
contain frequent ANSI escape sequences. Update the implementation to accumulate
output in a `[UInt8]` buffer during the state-machine pass, then convert it to
`Data` once at the end, preserving the existing `state` handling and `ESC
k`/`ST` title stripping behavior.
🪄 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: 790a5fad-06be-4029-ad4c-7851a29fa940
📒 Files selected for processing (4)
Sources/RemoteTmuxScreenTitleFilter.swiftSources/RemoteTmuxSessionMirror.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxScreenTitleFilterTests.swift
76891c3 to
d95b17a
Compare
…ew depends on it # Conflicts: # Sources/RemoteTmuxSessionMirror.swift
125af6e to
251054d
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
59f159a to
55ebddb
Compare
…ror output Implement the byte-stream filter in the CmuxRemoteSession package (pure logic, no app-lifecycle deps) rather than the app target root, matching the established pattern for lifted remote-session logic. Build into a [UInt8] buffer, and assert on raw Data in tests so a byte-corruption regression fails fast.
55ebddb to
537ebc8
Compare
Summary
A remote shell running inside tmux sees
TERM=screen*/tmux*, so its prompt (e.g. oh-my-zsh) sets the window title with the GNU screen escapeESC k <title> ESC \rather than the xterm OSC.%outputis the raw pty copy, so tmux forwards those bytes verbatim and only interprets them for its ownwindow_name— its rendered pane (capture-pane) has the title stripped. cmux's mirror surface is an xterm-style emulator that doesn't recognizeESC k, so it printed the title text onto the screen:Pre-existing in both the per-session and linked-view remote-tmux mirrors. Plain shells (no title-setting) were unaffected.
Fix
RemoteTmuxScreenTitleFilter— a per-pane, stateful byte filter (survives%outputchunk splits at any byte) applied inRemoteTmuxSessionMirror.routeOutput, droppingESC k … ESC \. It terminates only on ST (ESC \), never BEL, matching tmux/screen exactly (an unterminated title consumes until ST, just like tmux's own screen).ESC(the overwhelmingly common case) pass through with no copy/allocation.reseedAfterReconnect(synthetic clear/capture bytes) can't be swallowed by a filter left mid-title before the drop.Verification
Exhaustive battery diffing cmux's render (
read-screen) against tmux's ground truth (capture-pane) across 32 escape payloads — titles, OSC, DCS/APC/PM/SOS, SGR 16/256/truecolor, cursor/scroll/edit ops, erase, alt-screen, DEC charset, unicode/wide/emoji/zero-width — all render byte-identical (the only diffs arecapture-panevsread-screenserialization of tabs / DEC line-draw / zero-width, which are visually identical). Plus unit tests for the filter (chunk-split fuzzing, ST-only termination, passthrough of CSI/OSC/other escapes).Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Strip the GNU screen/tmux window-title escape
ESC k <title> ESC \from remote-tmux mirror output so title text no longer prints (e.g.,echoej). Matches tmux pane rendering and survives chunk splits.Bug Fixes
RemoteTmuxScreenTitleFilter, applied per-pane inRemoteTmuxSessionMirror.routeOutput; stateful across%outputsplits, terminates only on ST (never BEL), fast-paths chunks withoutESC, builds into a[UInt8]buffer, resets on disconnect, and prunes on pane removal. HookedonConnectionStateChangedto clear filter state when not connected.Data, covering split boundaries, BEL-not-terminating behavior, back-to-back titles, and passthrough of other escapes.Refactors
CmuxRemoteSessionpackage (pure logic) and moved mirror helpers intoRemoteTmuxSessionMirror+Helpers.swift. Updated the project to satisfy file-budget and package policy.Written for commit 537ebc8. Summary will update on new commits.
Summary by CodeRabbit
ESC k <title> ESC \) from mirrored terminal output, preserving correct behavior across chunk boundaries and per-pane streams.