Repository navigation
Conversation
|
@ejc3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
…etry recoverable A BatchMode=yes discovery probe through an ssh `ProxyCommand` whose own pre-handshake auth or 2FA leg silently aborts (no tty to prompt on) closes the proxy pipe before SSH emits any auth-failure string. Catch this stderr signature (OpenSSH's `to/by UNKNOWN port 65535` placeholder for pipe transports) and route it to the same interactive ssh retry already used for `Permission denied` / host-key TOFU / MFA. Introduces `indicatesProxyCommandTransportClosed` next to the existing `indicatesAuthRequired` and a composed `indicatesInteractiveRetryWillHelp` so the three RemoteTmuxController routing sites that previously each spelled out `indicatesAuthRequired` (mirrorHostInNewWindow's discovery catch, preflightControlAttach's catch arm, and authRequiredAttachArgv) now go through a single name — preventing the asymmetry where only one entrypoint would have gotten the new recovery and the others silently regressed.
… recoverable OpenSSH's `to/by UNKNOWN port 65535` placeholder is also emitted when a ProxyCommand / ProxyJump fails for reasons no interactive ssh retry can fix: target unreachable behind the jumphost, `nc` to a refused port, stdio forwarding teardown, target spoke no SSH on the negotiated port, DNS NXDOMAIN. Those failures stamp explicit diagnostic markers (`connect failed:`, `: open failed:`, `stdio forwarding failed`, `kex_exchange_identification:`, `Connection refused`, `No route to host`, etc.) into stderr alongside the placeholder; route them to the real error instead of bouncing the user through a futile interactive prompt. Tightens indicatesProxyCommandTransportClosed to require the placeholder AND no diagnostic marker before firing. Adds positive tests for the SILENT closures we still need to catch and negative tests for the EXPLAINED closures the predicate must now skip — including the precise stderr codex's review reproduced via `ssh -J nowhere.invalid` and `ssh -oProxyCommand='nc localhost 9'`.
…classification `xcodebuild test` against `cmuxTests/RemoteTmuxAuthTests` flagged that the `Could not resolve hostname …\nConnection closed by UNKNOWN port 65535` stderr slipped past the silent-closure check — OpenSSH wraps every `getaddrinfo` failure with that prefix (across macOS / Linux / Windows getaddrinfo strerrors) so the proxy DNS NXDOMAIN looked silent to the predicate. Adds `could not resolve hostname` to the non-recoverable marker list alongside the existing `name or service not known` / `temporary failure in name resolution` constants (which only cover the underlying getaddrinfo wording, not the OpenSSH wrapper).
…t-proxy-close Second codex review pass reproduced a remaining false positive: when a `ProxyCommand` uses `nc` and `nc` itself fails DNS, BSD/macOS netcat emits `nc: getaddrinfo: nodename nor servname provided, or not known` raw — OpenSSH's `Could not resolve hostname` wrapper only fires when OpenSSH does the resolution, not when an inferior `ProxyCommand` does. The resulting stderr has the proxy placeholder and no exclusion marker, so the predicate returned true and routed the user through a futile interactive retry. Adds `nodename nor servname provided` to the non-recoverable marker list and extends `doesNotClassifyExplainedProxyClosures` with the exact stderr codex reproduced via `ssh -o ProxyCommand='nc nonexistent.invalid 22'`.
…osures as non-recoverable Review follow-up: two more explained ProxyCommand/ProxyJump closures were slipping past the silent-closure check and routing the user through a futile interactive retry: - Linux TCP connect timeouts phrase it "Connection timed out" (only the BSD/macOS "Operation timed out" was covered), so an nc-based ProxyCommand timing out on Linux looked silent. - "ssh_exchange_identification:" banner-exchange closures (the inner target dropping the connection pre-auth: fail2ban, tcpwrappers, not-SSH-on-port) were only excluded when a second marker happened to co-occur. Adds both to nonRecoverableProxyMarkers with isolated negative tests (no co-present marker) so each is genuinely exercised.
First step of the linked-view transport for MaxSessions=1 hosts (2FA devservers), where only one concurrent SSH session is allowed so cmux cannot open a tmux -CC control client per remote session. The plan: attach ONE control client to a hidden, cmux-owned aggregate "view" session and link-window every mirrored session's windows into it, so all workspaces stream over the single connection while the real sessions stay intact for tmux ls / tmux attach interop. This commit lays the foundation, no behavior change yet: - `betaFeatures.remoteTmuxLinkedView` flag (off; requires remoteTmux). While off, remote tmux uses the existing per-session control connections unchanged. - `RemoteTmuxViewReconciler`: a PURE, deterministic diff of desired vs actual view contents into minimal link/unlink actions. Correctness is a reconciliation, not optimistic link-window side effects (per design review). Safety invariants: never unlink the view's placeholder window; never touch windows cmux did not link itself; idempotent; stable command ordering. - 9 unit tests covering link/unlink/idempotence and every safety invariant. Empirically de-risked over a real ssh + MaxSessions=1 harness (18/18 assertions: link→live %output for all linked windows, links survive abrupt transport loss, window-size manual holds independent per-window sizes, new-session-over-stream is the New Workspace fix, unlink-view-copy keeps the real session). Subsequent commits add the view connection, reconcile driver, sizing, lifecycle, and UI aggregation.
The linked-view transport creates a real, visible aggregate "view" tmux session per (host, cmux owner). Because it is remote state shared with the user's own tmux, cmux must own it precisely: never collide with another cmux install, always reattach its own across reconnect/relaunch, garbage-collect only its own stale views, and never touch a session it does not own or that a user created. `RemoteTmuxViewSession` (pure value type): - deterministic, sanitized session name (`cmux-view-<owner>`), - create commands that stamp `@cmux_view` / `@cmux_view_owner` / `@cmux_view_version` and use explicit `-x/-y` (no 80x24 placeholder flash), - `list-sessions -F` row parsing, - classification predicates: isOwnView / isOwnStaleView / isForeignView — the safety surface that bounds what cmux may reattach or collect. 15 unit tests (16 total in the suite incl. the reconciler) cover naming, option stamping, parsing, and that own/foreign are mutually exclusive and stale-collection never includes another owner's or a non-view session. Build + tests green.
In linked-view mode one -CC client carries windows from many home sessions (all linked into the view). cmux must regroup that flat window set back into per-session workspaces. `RemoteTmuxLinkedWorkspaceModel` (pure) turns `list-windows -a` rows into: ordered workspaces (home session -> its windows as tabs, by index), the desired-linked-window set fed to the reconciler, and window->home-session routing for %output. The view session and its placeholder are always excluded; view-only windows (no home session) are dropped. 6 unit tests. Build + tests green.
…tor) `RemoteTmuxLinkedViewPlan.plan(view:snapshot:)` composes the three tested layers into one pure decision: given list-sessions + list-windows -a snapshots and the view's current contents/ownership, it returns whether to create the view, which stale same-owner views to kill (never foreign), the reconcile link/unlink actions, and the resulting workspace grouping. The live coordinator becomes a thin I/O shell: snapshot over the stream → plan(...) → apply. 5 unit tests covering first run (create+link+group), steady state, new-workspace add, closed-session unlink, and stale-vs-foreign view handling. Build + tests green.
A codex adversarial pass on the pure decision core surfaced real safety holes before any live wiring. Fixes (+ regression tests for each): - Exclude ALL view sessions, not just our own: the workspace model takes the full set of view session names so a FOREIGN cmux install's `cmux-view-*` is never surfaced as a workspace nor its windows linked. (was: excluded only our name) - Collision-resistant view-session naming: append an FNV-1a/64 hash of the raw owner id so distinct owners (e.g. `a.b` vs `a:b`, which sanitize alike) never share a view name and fight over one session. - Name-prefix guard on view classification: isAnyView/isOwnView/isOwnStaleView/ isForeignView all require the reserved `cmux-view-` prefix, so a real session that merely has the `@cmux_view` option copied onto it can never be reused or killed as a view. - Deterministic single home per window: a window linked into multiple real sessions is attributed to the lexicographically smallest home, so it appears in exactly one workspace and %output routing is stable regardless of tmux row order. - Reconciler never empties the view: if unlinking would remove the view's last window (nil/stale placeholder), keep one linked — unlinking the last window kills the view session and churns the mirror. - Robust parsing: free-text session_name placed LAST in list formats and split with a maxSplits limit, so a separator inside a name can't drop/corrupt a row. All four suites green (37 tests). No live behavior yet; this hardens the core the live coordinator will build on.
… format-bump, FNV reuse) A high-effort /code-review pass on the hardened core found four more issues; all fixed with tests: - CROSS-HOST CORRECTNESS: the new list-* parsers used a non-printable (\u1f) field delimiter. The codebase already learned (RemoteTmuxSessionListParser) that tmux's utf8_sanitize() rewrites non-printable bytes to `_` for non-UTF-8 SSH clients (e.g. Amazon Linux 2023), which would collapse fields and silently drop EVERY row. Switched both view-session and window parsers to the printable `:` delimiter with the free-text name LAST and remainder-rejoin, matching the existing parser. + tests asserting the format carries no control byte and that a `:` inside a name is preserved. - CORRECTNESS (format bump): on a view formatVersion bump the stale view shares our name and is killed+recreated, but `actual` was computed from its windows so they were treated as present and never re-linked → empty view after upgrade. Plan now treats `actual` as empty whenever needsViewCreate, so all windows re-link into the fresh view. + regression test. - REUSE: extracted RemoteTmuxHost.fnv1a64Hex shared by connectionHash and the linked-view owner hash (was a duplicated FNV-1a/64 copy). - ROBUSTNESS: sanitizeOwner now strips newlines/control chars (was only CharacterSet.whitespaces), so an owner id with \n can't corrupt the session name. All four suites green.
…r bug /simplify pass (reuse/simplification/altitude): - Extracted RemoteTmuxSessionListParser.splitRows (shared tmux -F row splitter: newline split + last-field rejoin) and routed all three parsers through it (session-list, view-session, window model) — removes 3 copies of the split/CR-strip/rejoin skeleton and both ad-hoc fieldDelimiter constants. - workspaces(): collapsed the two-pass home resolution into one O(n) fold (was O(W*R) via per-window homeSession); same result, clearer. - Reconciler: named the never-empty-view guard (wouldEmptyView/keepAlive). Incidental correctness fix (pre-existing, surfaced by routing parse() through the shared splitter): the parser split lines with `split(separator: "\n")` and stripped `\r` via `line.last == "\r"`, but Swift clusters `\r\n` as ONE grapheme, so CRLF line endings were neither split nor stripped — leaving a trailing `\r\n` on the last field. splitRows now splits on `Character.isNewline` (matches `\n`, `\r`, and the `\r\n` grapheme), fixing the long-failing preservesNameWhitespaceAndStripsLineTerminator test. All 45 tests green.
…ew keystone) On MaxSessions=1 hosts the single -CC client holds the one allowed SSH session, so the linked-view coordinator cannot run list-sessions/list-windows -a as separate one-shot ssh commands — they must travel over the live control stream. Adds RemoteTmuxControlConnection.query(_:) async -> [String]?: sends a command and returns its %begin/%end reply body, correlated positionally via the existing pending-command FIFO (a new .query(UUID) command kind), exactly mirroring the .activityQuery pattern. Resolves nil on %error or when the stream becomes unusable (failPendingQueries() wired alongside failPendingActivityQueries at every teardown/reconnect/exit site) so an awaiting coordinator never hangs. Build green; no behavior change until the coordinator uses it.
RemoteTmuxViewConnection drives the linked-view transport for one host: - start(): pre-attach one-shots over the shared master (list-sessions to GC our own stale views, create the owned view at explicit -x/-y, record its placeholder window), then attach the single tmux -CC client to the view. - reconcile(): on connect and every %topology change, snapshot the server OVER the control stream via connection.query() (a 2nd one-shot ssh is refused under MaxSessions=1), run RemoteTmuxLinkedViewPlan.plan(), apply link/unlink, and publish the regrouped workspaces (serialized so overlapping events don't interleave). Unlink targets the view's copy by index so the real session keeps the window. - newWorkspace(): new-session -d over the stream + reconcile, so it links in and surfaces as a new workspace with no new SSH session — "new workspace rides the linking". View creation switched to raw arg-vectors (createArgvs) for the one-shot transport path (which quotes each token), replacing the shell-string form. Composes the tested pure layers; build + 37 tests green. Not yet wired to the controller/UI.
Adds an optional windowIdFilter to RemoteTmuxSessionMirror so several mirrors can share ONE control connection (the aggregate view session) and each render only its home session's tmux windows. nil (the default, per-session transport) is unchanged — renders every window. rebuild() and the close-tabs liveWindows set now iterate the filtered `mirroredWindowOrder`; updateWindowIdFilter(_:) re-scopes + rebuilds as the coordinator regroups (e.g. a new tab in this session). %output already self-filters via the per-panel maps, so no output routing change is needed. Build green; nil-filter path behavior-identical.
Behind betaFeatures.remoteTmuxLinkedView, host attach now drives the shared view connection instead of one control connection per session: - mirrorHostInNewWindow: after auth/discovery + window creation, if linkedView is on, startLinkedView() builds a RemoteTmuxViewConnection (per-install owner id persisted in UserDefaults) and returns; the per-session loop is skipped. - syncLinkedWorkspaces (the coordinator's onWorkspacesChanged): projects the regrouped workspaces into cmux workspaces + filtered SessionMirrors that all SHARE the host's single view connection — creates mirrors for new home sessions, re-scopes existing ones (a new tab adds a window id), removes ones whose session is gone, and drops the bootstrap workspace once a real one exists. - New Workspace in such a window routes to coordinator.newWorkspace() (new-session over the stream → links in → republishes) — "new workspace rides the linking". - Linked-view mirrors are constructed with managesOwnLifecycle:false so a shared- connection %exit doesn't trigger per-session teardown; the coordinator owns lifecycle (onEnded → teardownLinkedView closes the window). detachAll() stops coordinators + mirrors on quit (servers keep running for resume). SessionMirror gained the managesOwnLifecycle flag for this. Build green; the flag-off path (per-session transport) is unchanged.
Final adversarial pass (codex xhigh + review agents) on the complete linked-view feature surfaced lifecycle gaps; fix the contained ones: - Window close / app quit now tear down the shared `-CC` coordinator. `handleRemoteWindowClosed` and `killMarkedSessionsBeforeTerminate` only walked per-session `sessionMirrors`, so closing a linked-view window leaked the single shared SSH connection (teardownLinkedView was only reachable from the remote-stream-ended callback). New `stopLinkedView` helper, called from both paths, gated on LIVE coordinator state (not the beta flag, so toggling the flag off mid-session still tears down). - Quit-with-kill kills the host's real sessions over the live view stream (a one-shot ssh would be refused under MaxSessions=1) with a round-trip barrier so the kills land before the connection is stopped. - Closing a linked workspace kills its real session over the stream so the reconcile drops it; previously the workspace closed only locally and the next reconcile re-added the tab (session still alive). - Last real session gone -> tear down the view + dedicated window, since TabManager.closeWorkspace refuses to remove a window's final workspace. - New Workspace in a linked window gates on the ACTIVE workspace being a remote mirror, matching the non-linked path (a dragged-in local workspace stays local). - Shared-connection mirrors no longer drive the view connection's identity on %session-changed (same ownership gate as onExit). - View placeholder parsing is CRLF-safe; reused-view windows from a prior run are adopted as owned so reconcile can unlink dead linked copies.
The per-action handlers (new-tab, split, tab-split, close-tab, reorder,
rename-window, rename-workspace, paste, image-upload) only searched
`sessionMirrors`, so on a linked-view workspace they found nothing and
silently no-op'd ("inert handlers"). Linked mirrors live in a separate
`linkedMirrors` map sharing one view `-CC` connection.
Unify the lookup so every handler reaches a workspace's mirror in either
mode:
- `actionMirror(forWorkspaceId:)` / `actionMirrors` / `isLinkedMirror(_:)`.
- Most commands already address tmux's GLOBAL ids (`@windowId`, `%paneId`),
which route correctly over any connection to the server, so split,
tab-split, close-tab, rename-window, reorder, paste and image-upload work
over the shared view connection once they find the linked mirror.
- New-tab anchors on the home session's last window for linked mirrors
(the shared connection's `{end}` would resolve in the hidden view
session); non-linked keeps `{end}`, correct for its per-session stream.
- Rename-workspace targets the home session by name for linked mirrors
(the shared connection's session id is the view's, not the home's).
New accessor `RemoteTmuxSessionMirror.orderedWindowIds` exposes the
filtered window order for the new-tab anchor.
Live-testing on a MaxSessions=1 harness exposed a bug in the prior commit:
a new tab created in a linked-view workspace landed in the hidden VIEW
session, not the home session. A window linked into both the home session
and the view has an ambiguous `@id`, so `new-window -a -t @id` over the
shared view connection resolves to the view (the client's current session).
Fix: for linked mirrors, anchor `new-window` on the home session BY NAME
(`'<session>:{end}'`) so the tab lands in the right session and surfaces in
the correct workspace. Non-linked keeps window-id placement (its connection
is attached to that one session, so `@id` is unambiguous). Exact
after-panel placement is not preserved for linked tabs — correct session
beats exact position. Factor the shared `-c <wd>` tail into
`appendWorkingDirectory`.
Live-testing showed the new tab correctly landed in the home session but never appeared as a cmux tab: the new window isn't yet linked into the view, so the shared view stream receives no %window-add and nothing triggers a reconcile. Nudge the coordinator to reconcile after a linked new-tab (same pattern as new-workspace's new-session), so it links the window into the view and surfaces it as a tab.
In a remote-tmux window two browser entry points misbehaved on a mirror workspace (a 1:1 tmux view that refuses local browser surfaces): - "New Browser Workspace" (⌥⌘N / palette / menu) was intercepted by the remote new-workspace route and turned into a remote tmux session — the browser intent was lost. The remote route now fires only for TERMINAL new-workspaces; a browser one falls through and is created locally. - The globe tab-strip button (and "New Browser Tab") no-op'd, because newBrowserSurface refuses a mirror workspace. It now falls back to creating a new LOCAL browser workspace in the same window. The browser is always a local WebKit surface (never tunneled), so it lives as a normal local workspace alongside the window's remote mirrors.
The linked-view mode shares ONE `tmux -CC` client across every mirrored window, so the per-session `refresh-client -C` path can't size windows independently — they stayed at the view's fixed creation grid, clipping full-screen TUIs and ignoring cmux-pane resizes. Size each window explicitly instead: - The view session runs `window-size manual` so per-window sizes stick. - New `RemoteTmuxControlConnection.resizeWindow(windowId:columns:rows:)` sends `resize-window -t @id -x C -y R`, debounced (coalescing SwiftUI layout-settle oscillation) and re-applied on reconnect. - `RemoteTmuxSessionMirror` / `RemoteTmuxWindowMirror` gain `perWindowSizing` (set for linked mirrors): single-pane, multi-pane, and initial-sizing paths route to `resizeWindow` instead of `setClientSize`. Non-linked mirrors keep `refresh-client -C` (their client owns one session). Verified live on a MaxSessions=1 server: mirrored windows resize to the cmux grid (e.g. 99x35) instead of the default.
…irror entrypoints The mirror-workspace browser guard lived in `newBrowserSurface` (return nil) and was special-cased at only one entrypoint, so the globe tab-strip button and the command palette silently failed on a mirror workspace (a 1:1 tmux view that can't host a local browser pane). Centralize it at the shared `newBrowserSurface` chokepoint: on a mirror, an INTERACTIVE (`.userInitiated`) browser request now opens the browser in a NEW LOCAL workspace in the same window (via `addWorkspace(initialSurface: .browser)`, navigating to the URL or focusing the address bar) and returns its panel — so the globe button and the command palette behave identically. Removes the now-redundant per-entrypoint special-case in the tab-bar `.newBrowser` handler. Crucially the redirect is gated to `.userInitiated`. Restoration (`.restoration`) and automation/socket (`.automationPreload`) creation still return nil on a mirror (the long-standing "mirror refuses a local browser" behavior), because their callers require the returned panel to live in THIS workspace: restoration's replaceSurface would otherwise close the mirror's own pane and re-home it elsewhere, and the socket new-surface/browser handlers report the created surface under this workspace's id. A cross-workspace panel would corrupt them. (`performNewWorkspaceCreationAction` still gates the remote new-workspace route to `.terminal` so ⌥⌘N New Browser Workspace stays a local browser.)
`remoteTmux.linkedView.beta.enabled` had a catalog key and a runtime accessor but no Settings row, so it was only reachable by editing config. Add a "Remote tmux linked view" toggle to the Beta Features section, bound to the same catalog key the runtime reads, with English and Japanese strings. The toggle is disabled while "Remote tmux" is off, since linked view requires it.
…ndow The remote-tmux docs page didn't cover linked-view mode (one SSH connection for hosts that cap each connection to a single session, e.g. per-connection 2FA) or mirroring multiple hosts into one window. Add a "Linked view" section explaining the single-connection transport and how to enable it, plus a "Multiple servers in one window" subsection covering `--into-window`. English and Japanese message catalogs (the locales this page supports) updated.
Lets a second (or third) devserver's sessions aggregate into an existing linked-view window, so you can drive multiple servers from one cmux window — "new connection from another dev server expands the links." - RemoteTmuxWindowRegistry now maps a window to MULTIPLE hosts (each host still maps to one window). bind appends; unbind(hostHash:) removes just that host (dropping the window entry once empty); unbind(windowId:) clears all; adds hosts(forWindowId:). +5 unit tests. - mirrorHostInNewWindow gains `intoWindowId`: when it names a live linked-view window, the host's coordinator renders into that window's manager (no new window, no bootstrap workspace) instead of a fresh window. startLinkedView failure unbinds the host and only discards a window we created (never an aggregated one). - New Workspace / New Tab route to the SELECTED workspace's host coordinator (linkedViewCoordinator resolves via the selected mirror), so a new workspace rides the right server in a multi-server window. - Teardown is per-host: handleRemoteWindowClosed stops every aggregated host; killMarkedSessionsBeforeTerminate loops the window's hosts; teardownLinkedView closes only the ended host's workspaces and discards the window only when no other host remains. - Socket `remote.tmux.window` accepts `into_window` (window id) or `into_workspace` (resolved to its window); `cmux ssh-tmux --into-window <id|current>` exposes it (`current` uses the caller's CMUX_WORKSPACE_ID).
Second adversarial pass (codex xhigh + review agent) on the multi-server work surfaced these; fix the real ones: - New Workspace routing now gates on LIVE coordinator state, not the beta flag, so toggling the flag off mid-session still routes a new workspace to the selected workspace's host (was falling through to the first host or failing under MaxSessions=1). - Linked workspace rename nudges a reconcile so the mirror re-keys to the new session name; without it later new-tab/close targeted the old name. - startLinkedView failure now stops the half-started coordinator via stopLinkedView (was leaking it in linkedViews) and discards the window only when no other host has aggregated into it (a concurrent attach can join while we await) — previously it could tear down a second server's window. - Transport/ControlMaster teardown is guarded by a shared hostStillInUse check across BOTH modes, so tearing down a host's linked view doesn't pull the master out from under a per-session mirror of the same host (possible when the beta flag is toggled mid-session), and vice-versa. - Registry bind() drops a stale reverse entry when a host is re-bound to a different window, keeping windowIdByHost and hostsByWindowId consistent. - teardownLinkedView stops the coordinator before closing the host's workspaces, so the close can't re-enter the kill path. +1 registry test.
…pace The globe / new-browser button refused to work in a remote-tmux mirror workspace (newBrowserSurface returned nil), on the assumption that the mirror's rebuild() would reconcile away any non-tmux surface. It doesn't: the mirror reconcile is id-ownership based — it only adds/keeps/removes the surfaces it tracks in panelIdByWindow/panelIdByPane and never enumerates or prunes surfaces it doesn't own, and every routing decision (close/split/rename/reorder) keys off "is this a tmux window tab?" and falls through to local handling otherwise. So just remove the special case: a browser now opens as a normal local tab in the mirror workspace's pane, coexisting with the tmux-window tabs and surviving reconcile. This also makes the socket new-surface/browser handlers correct (the returned surface genuinely lives in the reported workspace) instead of needing a guard.
…a dedicated one "Multiple servers in one window" only aggregated into a window that was already a linked-view window; `cmux ssh-tmux <host> --into-window current` aimed at a regular main window fell through and opened a separate dedicated window instead. Relax the aggregation guard to also accept a window with no remote host bound yet (`!windowRegistry.isDedicatedWindow`), so a user can pull hosts into their main window alongside local workspaces. Per-session dedicated windows are still excluded (they keep the single-host invariant); aggregation still requires linked-view. Registry binding, window-close/quit teardown, and linkedViewCoordinator routing are already multi-host + mixed-content safe (they filter mirrors by workspace id and no-op for local workspaces), so no other change is needed.
…ndows When a window aggregates more than one origin (Local + one or more remote tmux hosts, or multiple hosts), draw a thin pastel color rail on each workspace row's left edge so it's clear which origin each workspace belongs to. A single-origin window shows no rail. - Origin = Local (a non-mirror workspace) or a specific remote host (resolved via the new RemoteTmuxController.remoteTmuxHost(forWorkspaceId:), keyed by connectionHash). Same host twice = same origin = same color. - Local = muted gray. Hosts = palette colors assigned in lexicographic order of destination, softened toward pastel, skipping the app accent color and any user-chosen custom workspace color so an auto color never collides. - Threaded as an immutable per-row value (WorkspaceListRenderContext -> TabItemView.originColor) to respect the sidebar snapshot-boundary rule; the row is Equatable on it. Reuses the existing left-rail capsule shape.
…spaces
When the selected workspace is a remote tmux mirror, prefix the window title with
the host destination ("host — workspace"). Hidden-titlebar main windows still
surface this in the Window menu / Mission Control, so windows that aggregate
remote sessions (incl. local + remote in one window) are identifiable. Local
workspaces are unaffected.
…review Adversarial review (code-review + codex) of the browser-as-tab and aggregate-into-a-regular-window changes surfaced three real issues: - HIGH: teardownLinkedView discarded the whole window when the last host left. For a host aggregated into the user's REGULAR window that also holds local workspaces, that destroyed local work. Now skip the discard when the window still has local (non-mirror) workspaces — matching the per-session path's existing ownedByEndingHost guard. - MEDIUM: a local browser tab in a mirror pane defeated tmux-driven tab reorder (mirrorTabReorder required the request to cover every tab). Now it reorders only the mirror tabs and pins non-mirror (local) tabs in place. - MEDIUM: splitting a local browser tab in a mirror was vetoed unconditionally. Now veto only when the split actually routed to tmux (a mirror window tab); a local browser tab splits locally. Also corrects the now-stale newBrowserSplit comment (split stays refused in a mirror because a second local pane would break the single-pane reorder invariant; tabs are allowed, splits are not).
…le-pane - Replace the vivid workspace-tag palette (whose first entry pastelized to a hot pink) with a dedicated, calm origin-rail palette — muted, distinguishable hues ordered quietest-first (soft blue, teal, sage, sand, lavender, ...). Lighter pastel blend since the palette is already muted. - Keep mirror workspaces single-pane: always veto a local split (a browser tab can't be split in a mirror; mirror-window-tab splits still route to tmux split-window). A second bonsplit pane would break the single-pane tmux-reorder invariant and could capture a newly-mirrored window — so disallow it rather than add fragile multi-pane placement logic.
…el too The host-name prefix for mirror workspaces was only applied to NSWindow.title (Window menu / Mission Control). cmux hides the native title bar and draws its own title (next to the folder drag icon) plus a "Cmd:" label, which still showed only the bare workspace name. Apply withRemoteHostPrefix (now internal) to both so a mirror workspace reads "host — workspace" in cmux's chrome as well.
…sign doc Replace the rail palette with the optimized "toned-down" set (clay, gold, green, teal, indigo, rose) — 6 muted hues chosen to maximize the minimum CIEDE2000 distance between every pair AND from the built-in colors (accent, selection blue, the grey/near-black backgrounds), with blue excluded so a rail never reads like the accent/selection. min pairwise ΔE ≈ 24.5; the rose is dusty, not a bright pink. Drop the pastel wash — these are tuned to read raw on both backgrounds. Adds docs/remote-tmux-origin-rail-palette.html, the comparison board used to pick the palette (mock sidebars on the real grey + near-black, none/all selected).
…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.
… servers in one window) Run from inside a cmux surface, ssh-tmux now aggregates the host into the caller's current window by default instead of always opening a new dedicated one. Adds --new-window to opt back into a fresh window; --into-window still targets one explicitly (the two are mutually exclusive). Outside a cmux surface it opens a new window as before.
…-loss fixes Mirror follows the remote session's active tmux window, but only on a structural change (a new mirror panel) or an actual active-window change — never re-selecting on a routine rebuild, which would override the user's manual tab choice on every command/resize. Window-lifecycle data-loss fixes (from the adversarial review): - teardownLinkedView no longer closes a mirror that was converted to an adopted local shell (guard on isRemoteTmuxMirror); - aggregating a host into a window clears the disposable mark on its local shells, so a later host-death teardown can't discard adopted work; - bounded re-bootstrap when an empty-host's freshly-created session vanishes; - forced close-window routes through the shared close path (CLI handle normalize + v2 sibling), and focus-window normalizes its handle too. (cherry picked from commit 5f259c1ee00c2ec36e7f7c0d81e689b8cb4db931)
ejc3
force-pushed
the
ejc3/remote-tmux-linkedview-fixes
branch
from
July 1, 2026 01:09
ec3cbd1 to
44a6a91
Compare
ejc3
changed the title
Contributor
Author
|
Closing for now. The linked-view lifecycle (mirror / disposable / kept-open state) is spread across several booleans and parallel registries — that caused a data-loss regression and turns every fix into another special case. Reworking it around a single explicit workspace state machine on a fresh branch; a clean replacement will follow. |
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.
Two problems I hit dogfooding linked-view mirrors.
A mirrored tab could take your work with it when it closed. Mirror a host, then let its remote tmux session end (or aggregate a second host into the same window). The now-dead mirror tab gets converted into a plain local shell so you can keep using it — you
cdsomewhere and start typing. Then the host's teardown runs, still treats that tab as a mirror, and closes it: your local shell and everything in it is gone.The teardown now checks whether the tab is still a live mirror before closing it (converted-to-local tabs are left alone). Aggregating a host into a window clears the "disposable" mark on that window's local shells so a later teardown can't discard them, and if an empty host's freshly-created session vanishes out from under us the tab is re-bootstrapped instead of left dead.
The mirror now tracks the remote's active window. Switch windows on the remote and cmux's active tab follows. The catch: a mirror rebuild fires on every command and every resize, so re-selecting on each rebuild would snap you back to the remote's active window and clobber a tab you clicked by hand. So re-selection only happens on an actual active-window change or when a new mirror tab appears — the linked-view
list-windowsformat carries#{window_active}and the model tracksactiveWindowIdto tell the difference.Also: programmatic
close-windownow takes the same close path as every other close, andclose-window/focus-windowaccept an index, ref, or UUID for the target window.The linked-view plan/model unit tests and the mirror render-fidelity harness pass.