cmux-tui manual-IO data path: pipe-io relay + reconnecting pump (no attach subprocess in the pane) - #10742
cmux-tui manual-IO data path: pipe-io relay + reconnecting pump (no attach subprocess in the pane)#10742lawrencecchen wants to merge 22 commits into
Conversation
stdout carries raw VT bytes (replay then live), stdin takes JSON input/ resize lines, stderr ends with one JSON exit reason, and exit codes separate terminal-ended (0) from daemon-lost (2) so an embedder knows whether to respawn. Also pins single reply authority: an inner DSR query is answered exactly once, by the daemon-side terminal.
The relay is the scoped attach client minus the renderer, for GUI surfaces running in Ghostty's manual-IO mode: a tap on the scoped session forwards the daemon's replay and live output bytes to stdout, stdin takes JSON input/resize lines, and a final stderr JSON line plus the exit code tell the embedder whether to respawn (0 = terminal ended or embedder closed stdin, 2 = daemon connection lost). Replays that are not the relay's first output are prefixed with a full reset because a replay replaces terminal state while a byte stream can only append. A full tap queue disconnects the transport instead of wedging the session reader thread. The daemon-side terminal stays the single reply authority; the relay parses nothing.
Behind terminal.beta.tuiBackend.manualIO (requires the backend flag): daemon-backed terminals run in Ghostty's manual-mirror IO mode with no surface command. TuiManualIOPump owns a cmux-tui attach --pipe-io relay per panel, feeds relay stdout into the surface, forwards the surface's encoded input bytes to relay stdin (the surface parsed the daemon terminal's own byte stream, so encodings always match the daemon's mode state and the exec bridge's mode-mirroring bug class cannot exist), and drives daemon-side sizing from applied surface resizes. Reconnects: relay exit 2 (daemon lost) keeps the last frame, shows the shared reconnect overlay (spinner, attempt count, Reconnect button), and respawns with 0.5s..30s backoff; every respawn resyncs by resetting the surface and replaying the daemon journal. Exit 0 (terminal ended) shows an informational ended overlay and never retries. Input while offline is dropped, never queued. The pump registry is keyed by surface id, so detach transfers need no bookkeeping; closing a panel stops its relay. Restore reattaches by tuiTerminalID through the same pump path and skips scrollback replay and agent auto-resume, like the exec bridge.
A terminal close tears the scoped attach stream down before the exit event reaches the relay, so stream loss alone cannot be reported as daemon-lost: the embedder would respawn against a dead terminal forever. On stream loss the relay now probes the daemon once (existing connection first, fresh connect if the socket died with the stream) and reports terminal-ended when the terminal no longer resolves. A pipe-io attach to an unknown terminal exits terminal-ended for the same reason. Resize outcomes are echoed as stderr diag lines (embedders skip lines without an exit key), and the resize e2e test polls stty because the daemon acknowledges before the PTY winsize lands.
Exit 2 without the relay's own JSON reason is a usage error (e.g. a cmux-tui binary without --pipe-io), not a daemon outage: it no longer reads as daemon-lost. Unexplained failures (usage errors, spawn failures, crashes) stop retrying after five consecutive attempts and show a distinct overlay pointing at the binary-path setting, with a manual Retry; explained daemon-lost exits keep retrying forever.
The daemon resizes a terminal's PTY only for its geometry-authority client; the full TUI client claims that for its active surface, and the relay never did, so resize-attached-view recorded the view size but the PTY stayed at the attach geometry (accepted=false). The relay now claims the authority right after attach. Also reap the adoptable hosts the exit-discrimination test deliberately orphans by SIGKILLing the daemon, so the fixture's leak check does not fail the arranged scene.
…ueue ghostty_surface_process_output blocks on libghostty's renderer/IO futex; calling it on the main thread wedged the app on the manual-IO tui path (sampled: main parked in ulock_wait inside the parse call with no holder running). iOS shipped this exact fix for its manual surfaces after 0x8BADF00D watchdog kills. processRemoteOutput now only enqueues: a per-surface serial TerminalRemoteOutputFeed makes the blocking call off-main in order, and every native-free site drains the feed before ghostty_surface_free, so queued or in-flight parses can never touch a freed surface. This also moves the existing remote-tmux mirror byte path off-main, which shared the same latent wedge.
…size A session-restored panel can mount at exactly the runtime's initial grid, so no applied-resize sample ever fires and the pump never spawned its relay (restored tabs stayed blank). The pump now flushes any pending manual-size report, samples the grid directly when the runtime is or becomes ready, and attaches eagerly; a later mount only resizes the daemon terminal, matching the exec bridge's spawn-now-resize-later behavior.
A relay spawned while the daemon socket is down exited 1 with no machine-readable reason, which the pump counts as an unexplained failure; five of those during one daemon restart parked the pane in the terminal failed state before the daemon came back. Startup connect, scope, and tree-refresh failures in pipe-io mode now exit 2 with the daemon-lost reason (retry forever), pinned by a dead-socket e2e test. The pump also cancels any pending retry when entering failed and refuses to spawn from terminal states except through the overlay's Retry, which re-enters reconnecting first.
Process termination is not pipe EOF: the pump read the stderr box before the relay's final exit-reason line landed, so a genuine daemon-lost exit classified as an unexplained failure and a daemon restart could still park panes in the failed state. The pump now drains stderr to EOF on a throwaway queue (continuously, so a full pipe can never wedge the relay) and classifies once both the exit status and the drain have arrived for the current generation.
The app-host test target compiles the pump tests against the package type TerminalRemoteOutputFeed; the missing import failed all four app-host shards. enqueue is public so the lifetime test can exercise the closed gate directly.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a renderer-less
Confidence Score: 3/5The PR should not merge until hibernation preserves a usable output feed and every relay replacement reliably retires the previous process. Reversible runtime teardown permanently disables future remote output, while reconnect and overflow paths can launch replacement relays without terminating the currently running process. Files Needing Attention: Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swift, Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalRemoteOutputFeed.swift, Sources/TuiManualIOPump.swift Important Files Changed
Sequence DiagramsequenceDiagram
participant Surface as Ghostty manual surface
participant Pump as TuiManualIOPump
participant Relay as cmux-tui pipe-io
participant Daemon as Session daemon
Surface->>Pump: encoded input / applied resize
Pump->>Relay: JSON stdin line
Relay->>Daemon: input or resize
Daemon-->>Relay: replay then live VT bytes
Relay-->>Pump: stdout byte stream
Pump-->>Surface: processRemoteOutput
alt daemon connection lost
Relay-->>Pump: exit 2 / daemon-lost
Pump->>Relay: respawn after backoff
Relay->>Daemon: reattach and request replay
end
Reviews (1): Last reviewed commit: "fix(tests): import CmuxTerminal for the ..." | Re-trigger Greptile |
| isolatedHibernationReservation: teardownReservation | ||
| isolatedHibernationReservation: teardownReservation, | ||
| freeSurface: { surface in | ||
| remoteOutputFeed.drainAndClose() |
There was a problem hiding this comment.
Hibernation permanently closes output feed
When a remotely fed surface resumes after agent hibernation, this teardown has permanently closed the immutable remoteOutputFeed, so the recreated runtime silently drops all subsequent TUI or remote-tmux output and remains blank or stale.
Knowledge Base Used: Ghostty terminal integration
| private func spawnRelay() { | ||
| guard !stopped, let surface else { return } |
There was a problem hiding this comment.
Relay replacement leaves orphan process
When stdout overflows, or Reconnect is pressed after a relay spawns but before its first output, the replacement path launches another relay without closing or terminating the existing process. The old relay remains attached with open pipes, leaking resources and allowing stale input or geometry updates to reach the wrong relay.
Knowledge Base Used: TUI client and relays
| retryTask?.cancel() | ||
| retryTask = Task { @MainActor [weak self] in | ||
| do { | ||
| try await sleep(delay) |
There was a problem hiding this comment.
Retry lifecycle uses direct sleep
The production reconnect transition is driven directly by Task.sleep, embedding timing and cancellation coordination in the pump rather than the repository-required cancellation-aware retry abstraction or owning subsystem signal.
Rule Used: Flag new blocking or timing-based synchronization ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /// surface they render. | ||
| @MainActor | ||
| final class TuiManualIOPumpRegistry { | ||
| static let shared = TuiManualIOPumpRegistry() |
There was a problem hiding this comment.
Pump ownership becomes ambient state
This singleton stores all live pumps and makes panel creation, reconnect, overlay lookup, and cleanup depend on process-global mutable state. Owning and injecting the registry through the surface or workspace lifecycle would keep pump ownership explicit and tests isolated.
Rule Used: Flag new ambient global state in production Swift:... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…-tui-manual-io # Conflicts: # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Input.swift # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swift # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift # Resources/Localizable.xcstrings # Sources/Workspace.swift # cmux-tui/crates/cmux-tui-core/src/terminal_host_protocol.rs # cmux-tui/crates/cmux-tui/src/main.rs
…ock) The origin/main merge committed raw conflict markers and a stale duplicate key block into Localizable.xcstrings. Rebuild the file as the spike branch's merged catalog plus the 9 keys this branch adds (settings.betaFeatures.tuiTerminalBackendManualIO* and tui.overlay.*). The 18 duplicated keys that remain are pre-existing on main.
The interrupted merge committed raw conflict markers in three CmuxTerminal surface files. Main landed its own off-main remote-output path (TerminalSurfaceRemoteOutputLane, drained via the teardown coordinator's beforeFree fence), which supersedes this branch's TerminalRemoteOutputFeed deadlock fix. Resolve every hunk to the lane, delete the feed and its lifetime test, and route pipe-io bytes through processRemoteOutput onto the lane.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
…nals The scoped attach client minus the renderer, for an embedder that parses terminal bytes itself: stdout carries the daemon replay then live output (full reset before any non-first replay), stdin takes JSON input/resize lines, stderr ends with one machine-readable exit reason, and exit codes distinguish terminal-ended (0, do not respawn) from daemon-lost (2, respawn to resync from a fresh replay). On stream loss the relay probes the daemon once to tell a closed terminal from a daemon outage. The daemon-side vt stays the single reply authority (the client mirror has no on_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Extracted from feat-tui-manual-io (PR #10742) with the resource-boundary lint fixed (no 'surface' in public help text) and the flag documented in spec/cli.md.
Manual-IO data path for the cmux-tui terminal backend, stacked on the exec-attach spike (#10408). Implements step 1 of the migration plan's Manual-IO surface section: the daemon-backed surface stops running
cmux-tui attachas its command; the app pumps bytes directly into a manual-mirror Ghostty surface through a newattach --pipe-iorelay.Rust relay (
cmux-tui attach --terminal <id> --pipe-io [--cols N --rows N]): a tap on the scoped session forwards the daemon replay and live output to stdout; stdin takes JSONinput/resizelines; stderr ends with one JSON exit reason; exit 0 = terminal ended or embedder closed stdin (never respawn), exit 2 = daemon lost (respawn to resync). Non-first replays are prefixed with a full reset because a replay replaces state while a byte stream appends. On stream loss the relay probes the daemon once to distinguish a closed terminal from a daemon outage, and startup connect failures report daemon-lost so a restart window retries. The relay claims the terminal geometry authority so embedder resizes drive the PTY. The daemon-side vt stays the single reply authority (client mirror has noon_pty_write), pinned by an e2e test asserting an inner DSR query is answered exactly once. Red-first: 0e81167 adds the failing e2e contract tests, later commits go green (run 32852935465, Linux + macOS).Swift pump (
terminal.beta.tuiBackend.manualIO, requires the backend flag, default off):TuiManualIOPumpowns one relay per panel, feeds stdout into the surface, forwards the surface's encoded input to relay stdin, and drives daemon sizing from applied resizes. Because the surface parses the daemon terminal's own byte stream, input encodings always match the daemon's mode state: the exec bridge's mode-mirroring bug class (rounds 2-6) cannot exist here, and no shell wrapper orenv(1)prefix exists on the surface path. Reconnects: connecting -> live -> reconnecting(n) -> live | ended | failed; backoff 0.5s..30s cap; explained daemon-lost exits retry forever; five consecutive unexplained failures park in afailedoverlay with manual Retry; respawn resyncs by resetting the surface and replaying the journal; offline input is dropped, never queued. The reconnect UI reuses the cloud terminal overlay (spinner + attempt count + Reconnect; informational for ended/failed; localized en+ja). Restore reattaches by persistedtuiTerminalIDthrough the same pump and suppresses scrollback replay and agent auto-resume.Deadlock found, then superseded by main's fix:
ghostty_surface_process_outputblocks on libghostty's renderer/IO futex; calling it on the main thread (asprocessRemoteOutputdid) wedged the app on the first replay feed (sampled: main parked inulock_wait2, socket commands piling up in dispatch_sync-to-main). This branch originally fixed it with a per-surface serialTerminalRemoteOutputFeed; main then landed its own off-main path (TerminalSurfaceRemoteOutputLane, drained through the teardown coordinator'sbeforeFreefence). After merging main, this PR adopts the lane everywhere and deletes the feed, so pipe-io bytes ride the same reviewed mechanism as remote tmux output.Preflighted end to end on a tagged build (tuimio): live rendering and typing round-trip through the daemon, SIGKILL+relaunch reattaches with replayed content, daemon kill -> reconnecting with paused input -> daemon restart -> automatic resync with content intact, terminal close -> ended state, and the exec path is byte-identical with the flag off.
Known cuts (same scope as the spike): only new terminal tabs and restored panels are daemon-backed (splits and the first workspace terminal keep the local path); Ghostty-side scrollback reflow on resize is left at the manual-IO default; the reconnect overlay's pixel appearance was verified at the policy/state level (unit-tested presentation + debug-log states), not by screenshot, because the throwaway window sits off the active Space.