Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNote 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 beta-gated remote-tmux multiplexing over shared host connections, per-session channels, deterministic reconciliation, workspace lifecycle routing, remote workspace creation, and parser, integration, fuzz, and FIFO test coverage. ChangesRemote tmux multiplexing
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AppDelegate
participant RemoteTmuxController
participant RemoteTmuxViewConnection
participant RemoteTmuxMultiplexReconciler
participant RemoteTmuxSessionChannel
AppDelegate->>RemoteTmuxController: intercept remote terminal workspace request
RemoteTmuxController->>RemoteTmuxViewConnection: create session over shared view stream
RemoteTmuxViewConnection-->>RemoteTmuxController: publish regrouped workspace
RemoteTmuxController->>RemoteTmuxMultiplexReconciler: reconcile published sessions
RemoteTmuxMultiplexReconciler-->>RemoteTmuxController: create or update mirror plan
RemoteTmuxController->>RemoteTmuxSessionChannel: configure scoped session mirror
Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (6 errors, 1 warning)
✅ Passed checks (18 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@cmuxTests/RemoteTmuxMultiplexFuzzTests.swift`:
- Around line 292-295: Update the leak-tracking logic using leakProbes and
seenMirrorIDs so ObjectIdentifier values are only used to deduplicate mirrors
while they are alive, not across the entire test. Remove or reset stale
identifiers when their corresponding LeakProbe/mirror is deallocated, ensuring a
newly allocated mirror at a reused address receives a probe and remains covered
by assertNoLeaks.
In `@cmuxTests/RemoteTmuxWindowReorderTests.swift`:
- Around line 82-89: Update drainLeadingOther in the .paneRects response to
derive the pane ID from the requested window ID using the staged layout
convention (windowId * 10), rather than always replying with %0; preserve the
existing pane rectangle fields and ensure windows 2 and 3 receive their
corresponding pane IDs.
In `@Sources/Debug/RemoteTmux/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 695-701: The publicationReady calculation must consult the
RemoteTmuxSessionChannel’s underlying pending-work state rather than treating
non-RemoteTmuxControlConnection sources as ready. Update the logic around
publicationReady to use the channel settlement signal, including queued
listWindows, paneRects, and sizing work, and return false when the source or
signal is unavailable.
In `@Sources/RemoteHostColorRegistry.swift`:
- Around line 33-47: Remove the static shared instance from
RemoteHostColorRegistry and make the registry owned by an injectable
application-level owner such as AppDelegate or the remote-tmux coordinator. Pass
that instance into snapshot construction and all consumers requiring host-color
assignments, preserving one consistent process-wide registry without introducing
another global runtime owner.
In `@Sources/RemoteTmuxController`+Decisions.swift:
- Around line 73-92: The multiplexed new-window handling in the decision
overloads ignores afterWindowId and can target the hidden view session. Update
newWindowCommandInSession and both callers to accept and pass the optional
window target, routing all overloads through this single session-scoped action
path while preserving focus and reconciliation behavior.
- Around line 463-471: Update the dedicated-session creation flow around
transport(for:).runTmux and the created guard to distinguish ambiguous transport
or timeout errors from confirmed failures: return .createIndeterminate when
execution may have reached tmux, and reserve .createFailed for confirmed
pre-execution or command failures. Preserve successful session-name handling and
avoid retrying indeterminate outcomes.
In `@Sources/TerminalController`+RemoteTmux.swift:
- Around line 37-44: The new remote workspace route must not use the blocking
v2VmCall wrapper. Update the route around createRemoteWorkspace to use the
existing asynchronous response mechanism, preserving the current controller
lookup and outcome handling while allowing the socket worker to process
unrelated commands during workspace creation.
🪄 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: 7b0bbea9-d6a3-449f-aaad-8df0edf877c7
📒 Files selected for processing (33)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/RemoteHostColorRegistry.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+PaneSubscriptions.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController+Attach.swiftSources/RemoteTmuxController+Decisions.swiftSources/RemoteTmuxController+Multiplexer.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/RemoteTmuxLinkedViewPlan.swiftSources/RemoteTmuxLinkedWorkspaceModel.swiftSources/RemoteTmuxMultiplexReconciler.swiftSources/RemoteTmuxSessionChannel.swiftSources/RemoteTmuxSessionListParser.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxSessionSource.swiftSources/RemoteTmuxViewConnection.swiftSources/RemoteTmuxViewReconciler.swiftSources/RemoteTmuxViewSession.swiftSources/RemoteTmuxWindowMirror.swiftSources/SidebarWorkspaceSnapshotFactory.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swiftcmuxTests/RemoteTmuxLinkedWorkspaceModelTests.swiftcmuxTests/RemoteTmuxMirrorTargetingTests.swiftcmuxTests/RemoteTmuxMultiplexFuzzTests.swiftcmuxTests/RemoteTmuxWindowReorderTests.swift
Greptile SummaryThis PR introduces a beta remote tmux multiplexer transport that routes all of a host's sessions through one shared
Confidence Score: 4/5Safe to merge after fixing the unconditional ownedWindowIds.remove in the unlinkFromView reconcile case. The PR is architecturally well-structured with pure computation layers, correct actor isolation, and thorough localization. One P1 bug was found: the unlinkFromView branch drops ownedWindowIds unconditionally on send failure, which can leave ghost links in the view session that will never be retried. All other patterns — queryWithTimeout, CheckedContinuation deadline tasks, two-phase rename, nilIdWindowMatches guard, deferFullPaneReseed replacement — are correct. Files Needing Attention: Sources/RemoteTmuxViewConnection.swift — the unlinkFromView ownership-tracking bug must be fixed before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as Socket Client
participant TC as TerminalController
participant RC as RemoteTmuxController
participant VC as RemoteTmuxViewConnection
participant Host as SSH Host
CLI->>TC: "remote.tmux.new_workspace {workspace_id, name}"
TC->>RC: createRemoteWorkspace(referenceWorkspaceId:name:)
RC->>RC: newSessionHost() — resolve host from active mirror
RC->>VC: createWorkspaceReturningName(named:)
VC->>Host: new-session -d -P -s name (via shared -CC stream)
Host-->>VC: session name (queryWithTimeout 10s)
VC-->>RC: .created(sessionName)
RC->>RC: awaitNewWorkspace(host:sessionName:deadline:)
Note over RC: CheckedContinuation — waits for session-digest notify or timeout
Host-->>RC: %notify cmux_sessions (refresh-client -B)
RC->>RC: applyMultiplexedWorkspaces → create mirror
RC-->>TC: .created
TC-->>CLI: "{status: ok, workspace_id: ...}"
Reviews (11): Last reviewed commit: "remote-tmux: route pane seeds and diagno..." | Re-trigger Greptile |
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Sources/TerminalController+RemoteTmux.swift (1)
24-70: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftBlocking
DispatchSemaphorewait re-surfaces the previously flagged worker-stall risk.
v2VmCall(line 40) blocks the calling thread onsemaphore.wait(timeout:)for up to 45s whilecreateRemoteWorkspaceperforms SSH/tmux round trips. This exact hazard was raised on an earlier revision of this same method and was not addressed — siblingremote.tmux.*methods already use the same blocking wrapper, but this is a new call site on a path with real network latency, materially expanding the exposure.#!/bin/bash # Description: Confirm whether socketWorkerV2Response runs on a shared worker # (blocking impacts all connections) or a per-connection dedicated thread # (blocking only serializes that connection's own requests). rg -n "socketWorkerV2Response|socketWorkerMethods|socketWorkerCoordinatorHopMethods" Sources/TerminalController.swift -A3 -B3 | head -100🤖 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 `@Sources/TerminalController`+RemoteTmux.swift around lines 24 - 70, Remove the blocking v2VmCall wrapper from v2RemoteTmuxNewWorkspace and expose the asynchronous createRemoteWorkspace flow through the nonblocking worker/coordinator mechanism used by the existing remote.tmux methods. Preserve the current validation, 45-second operation deadline, outcome mapping, and error responses while ensuring no DispatchSemaphore wait blocks the socket worker during SSH/tmux operations.Source: Coding guidelines
🤖 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 `@Sources/Debug/RemoteTmux/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 695-710: Remove the `?? true` fallback in the
`RemoteTmuxSessionChannel` branch of the `publicationReady` switch. Ensure an
unrecognized `channel.underlying` fails closed with `publicationReady = false`,
while a `RemoteTmuxControlConnection` continues to use
`hasPendingSizingSettlementWork(windowId:)`.
In `@Sources/RemoteTmuxControlConnection.swift`:
- Around line 945-947: Reset sessionDigestSubscribed alongside the other
client-owned subscription state in beginReconnecting(). Ensure the reconnect
path allows the existing isSharedViewStream branch to call
subscribeSessionDigest() again after the new client attaches, while preserving
normal subscription behavior.
In `@Sources/RemoteTmuxController`+Decisions.swift:
- Around line 144-165: Extract the shared working-directory validation and `-c`
argument construction from `newWindowCommandInSession` and `newWindowCommand`
into a single helper, then have both methods append the helper’s result.
Preserve the existing trimming, `controlModeLineSafeName` validation, quoting,
and omission behavior for nil, empty, or unsafe directories.
---
Duplicate comments:
In `@Sources/TerminalController`+RemoteTmux.swift:
- Around line 24-70: Remove the blocking v2VmCall wrapper from
v2RemoteTmuxNewWorkspace and expose the asynchronous createRemoteWorkspace flow
through the nonblocking worker/coordinator mechanism used by the existing
remote.tmux methods. Preserve the current validation, 45-second operation
deadline, outcome mapping, and error responses while ensuring no
DispatchSemaphore wait blocks the socket worker during SSH/tmux operations.
🪄 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: b67c2bee-3c7f-4f00-92dd-a72272be9826
📒 Files selected for processing (30)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+PaneSubscriptions.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController+Attach.swiftSources/RemoteTmuxController+Decisions.swiftSources/RemoteTmuxController+Multiplexer.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/RemoteTmuxLinkedViewPlan.swiftSources/RemoteTmuxLinkedWorkspaceModel.swiftSources/RemoteTmuxMultiplexReconciler.swiftSources/RemoteTmuxSessionChannel.swiftSources/RemoteTmuxSessionListParser.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxSessionSource.swiftSources/RemoteTmuxViewConnection.swiftSources/RemoteTmuxViewReconciler.swiftSources/RemoteTmuxViewSession.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/RemoteTmuxLinkedWorkspaceModelTests.swiftcmuxTests/RemoteTmuxMirrorTargetingTests.swiftcmuxTests/RemoteTmuxMultiplexFuzzTests.swiftcmuxTests/RemoteTmuxWindowReorderTests.swift
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
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. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swift (1)
705-707: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
?? truefallback re-introduces the "default to ready" defect one level deeper.The outer switch correctly fails closed (
default: publicationReady = false), but theRemoteTmuxSessionChannelcase still defaults totrueifchannel.underlying as? RemoteTmuxControlConnectionfails. As per path instructions, correctness-critical state must fail closed when the reliable signal is missing. This is the same "unknown source defaults to settled" pattern flagged previously, just moved inside the case body.🐛 Proposed fix
case let channel as RemoteTmuxSessionChannel: - publicationReady = (channel.underlying as? RemoteTmuxControlConnection) - .map { !$0.hasPendingSizingSettlementWork(windowId: windowId) } ?? true + guard let underlyingConnection = channel.underlying as? RemoteTmuxControlConnection else { + publicationReady = false + break + } + publicationReady = !underlyingConnection.hasPendingSizingSettlementWork(windowId: windowId)🤖 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 `@Sources/Debug/RemoteTmux/TerminalController`+RemoteTmuxTestSupport.swift around lines 705 - 707, Update the RemoteTmuxSessionChannel case in the publicationReady logic to fail closed when channel.underlying cannot be cast to RemoteTmuxControlConnection: replace the ?? true fallback with false while preserving the existing hasPendingSizingSettlementWork check for valid control connections.Source: Path instructions
🤖 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.
Duplicate comments:
In `@Sources/Debug/RemoteTmux/TerminalController`+RemoteTmuxTestSupport.swift:
- Around line 705-707: Update the RemoteTmuxSessionChannel case in the
publicationReady logic to fail closed when channel.underlying cannot be cast to
RemoteTmuxControlConnection: replace the ?? true fallback with false while
preserving the existing hasPendingSizingSettlementWork check for valid control
connections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7505fe2-8464-4c33-a656-898e87d2c68d
📒 Files selected for processing (31)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+PaneSubscriptions.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController+Attach.swiftSources/RemoteTmuxController+Decisions.swiftSources/RemoteTmuxController+Multiplexer.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/RemoteTmuxLinkedViewPlan.swiftSources/RemoteTmuxLinkedWorkspaceModel.swiftSources/RemoteTmuxMultiplexReconciler.swiftSources/RemoteTmuxSessionChannel.swiftSources/RemoteTmuxSessionListParser.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxSessionSource.swiftSources/RemoteTmuxViewConnection.swiftSources/RemoteTmuxViewReconciler.swiftSources/RemoteTmuxViewSession.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swiftcmuxTests/RemoteTmuxLinkedWorkspaceModelTests.swiftcmuxTests/RemoteTmuxMirrorTargetingTests.swiftcmuxTests/RemoteTmuxMultiplexFuzzTests.swiftcmuxTests/RemoteTmuxWindowReorderTests.swift
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Sources/RemoteTmuxController.swift`:
- Around line 869-871: Update killMarkedSessionsBeforeTerminate(timeout:) so
awaitCommandBarrier uses the method’s passed timeout instead of hardcoded 3
seconds, applying the existing Duration-to-seconds conversion used elsewhere.
🪄 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: 4799a69c-af2e-4ab5-8b0e-39c43f9ae46a
📒 Files selected for processing (25)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+PaneSubscriptions.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController+Attach.swiftSources/RemoteTmuxController+Decisions.swiftSources/RemoteTmuxController+Multiplexer.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/RemoteTmuxLinkedViewPlan.swiftSources/RemoteTmuxLinkedWorkspaceModel.swiftSources/RemoteTmuxMultiplexReconciler.swiftSources/RemoteTmuxSessionChannel.swiftSources/RemoteTmuxSessionListParser.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxSessionSource.swiftSources/RemoteTmuxViewConnection.swiftSources/RemoteTmuxViewReconciler.swiftSources/RemoteTmuxViewSession.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swift
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
manaflow-ai#8428 and manaflow-ai#7193 each add an identical hostDestination(forWorkspaceId:) standalone, which only collides when both land. Keep one. At real merge time the second PR to land drops its copy; this commit lives only on the dogfood roll-up.
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 `@Sources/RemoteTmuxViewReconciler.swift`:
- Around line 98-108: Update sortedByNumericId to use a Schwartzian transform:
map each ID once into a decorated value containing the original string and
numericWindowId result, sort the decorated collection using those cached values
with the existing numeric and lexicographic ordering, then map back to the
original IDs. Remove numericWindowId calls from the sorting closure.
- Around line 117-122: Update the placeholder-only check in the surrounding
reconciler method to avoid copying actualWindowIds into nonPlaceholder. Return
true using an O(1) count/contains condition: the set must contain exactly one
element and, when present, that element must be placeholderWindowId; preserve
the behavior for a missing placeholder and other window IDs.
- Line 84: Replace the allocating
actualWindowIds.subtracting(unlinkable).isEmpty check in the wouldEmptyView
computation with an O(1) count comparison between actualWindowIds and
unlinkable, preserving the existing subset assumption and boolean 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: 11624857-6a79-404a-8eaf-bbe449b83668
📒 Files selected for processing (31)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftSources/AppDelegate.swiftSources/Debug/RemoteTmux/TerminalController+RemoteTmuxTestSupport.swiftSources/RemoteTmuxControlCommandKind.swiftSources/RemoteTmuxControlConnection+CommandResults.swiftSources/RemoteTmuxControlConnection+PaneSubscriptions.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController+Attach.swiftSources/RemoteTmuxController+Decisions.swiftSources/RemoteTmuxController+Multiplexer.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/RemoteTmuxLinkedViewPlan.swiftSources/RemoteTmuxLinkedWorkspaceModel.swiftSources/RemoteTmuxMultiplexReconciler.swiftSources/RemoteTmuxSessionChannel.swiftSources/RemoteTmuxSessionListParser.swiftSources/RemoteTmuxSessionMirror.swiftSources/RemoteTmuxSessionSource.swiftSources/RemoteTmuxViewConnection.swiftSources/RemoteTmuxViewReconciler.swiftSources/RemoteTmuxViewSession.swiftSources/RemoteTmuxWindowMirror.swiftSources/TerminalController+RemoteTmux.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swiftcmuxTests/RemoteTmuxLinkedWorkspaceModelTests.swiftcmuxTests/RemoteTmuxMirrorTargetingTests.swiftcmuxTests/RemoteTmuxMultiplexFuzzTests.swiftcmuxTests/RemoteTmuxWindowReorderTests.swift
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
|
Rebasing this onto current
I have fixed that part: the protocol now returns What is left is bigger than a signature.
So the multiplexer work needs the seed-ordering change from #8436 folded into it properly before this is mergeable again. The rebase itself is verified otherwise — all 24 commits preserved, the project file union checked against the repo's own pbxproj gates — and it is parked on the fork at |
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
|
All contributors have signed the CLA ✍️ ✅ |
manaflow-ai#8721 is the integration branch for the remote-tmux transport line. Its branch now also carries the later commits of manaflow-ai#8428 (the session multiplexer), manaflow-ai#8556 (the transport seam) and manaflow-ai#8555 (the reconnect login), cherry-picked with their conflicts resolved, so this one merge brings in all four. Conflicts against the roll-up, and how each was resolved: - RemoteTmuxConnectionState.swift, RemoteTmuxControlConnection.swift, RemoteTmuxController+Attach.swift, RemoteTmuxAuthTests.swift: only manaflow-ai#8555 touched these on the roll-up side. The roll-up's copy equals manaflow-ai#8555's head, and a three-way merge with manaflow-ai#8555's head as the base comes out identical to manaflow-ai#8721's file, so manaflow-ai#8721's version is taken. - RemoteTmuxController+Decisions.swift and RemoteTmuxNewWorkspaceHostRoutingTests.swift: manaflow-ai#8721's copies contain manaflow-ai#7214's routing and tests, plus the multiplexed-host path through routeMirrorNewWindow. manaflow-ai#8721's versions are taken. - RemoteTmuxWindowMirror+Configuration.swift: manaflow-ai#11248 and manaflow-ai#8721 both drop the pane tab bar in a single-pane mirror window. The only difference was manaflow-ai#11248's `nonisolated` on paneTabBarVisibility, which is kept. - BetaFeaturesCatalogSection.swift: both flags are kept, manaflow-ai#7193's remoteTmux.originColors and manaflow-ai#8721's remoteTmux.multiplexer. - AppDelegate.swift: the New Workspace routing check keeps manaflow-ai#7214's `!forceLocal`, so New Local Workspace still creates a local workspace. - RemoteTmuxController.swift: one copy of each New Workspace member. The routing is manaflow-ai#8721's, with the multiplexed in-band create and the readiness drop, but it reads the host through manaflow-ai#7214's newSessionHost helper, which wouldNewWorkspaceSpawnRemote also uses, and revalidates against registered main-window contexts as manaflow-ai#7214 does. The failure alert is manaflow-ai#8721's. The host lookups are manaflow-ai#7193's hostDestination and hostDestinationsByWorkspaceId. detachAll takes manaflow-ai#8721's side, which also stops every multiplexed host's shared view stream. The roll-up's explicit selectWorkspace is dropped, because manaflow-ai#8721 passes `select:` when it creates the workspace. - project.pbxproj: both routing test files stay registered. A second group entry for RemoteTmuxNewWorkspaceHostRoutingTests.swift, left over from the merge, is removed. - Localizable.xcstrings: the roll-up's catalog, with manaflow-ai#8721's entries for cli.help.ssh-tmux, common.ok and the two New Workspace dialog strings, which have all 20 locales and the new message text, plus manaflow-ai#8721's six new keys. Checked by parsing the result against the expected key set, 6571 keys. - scripts/lint-remote-tmux-no-polling.sh: manaflow-ai#11264's script, with its per-wait baseline keys, counted allowances and failing closed on a broken scan, plus manaflow-ai#8721's allowlist of deadline arms. All 13 allowlisted functions exist in the tree. The baseline was regenerated from the merged sources, and it matches manaflow-ai#11264's five entries. - scripts/remote-tmux-et-conformance-selftest.sh: six lines from manaflow-ai#8721 ended in a space. The whitespace is stripped here and on manaflow-ai#8721's branch. Checked on this tree: lint-remote-tmux-no-polling ok (13 documented, 5 baselined), lint-remote-tmux-no-polling.test.sh 12 passed, localization parity 0 errors, xcstrings lint passed, pbxproj test wiring ok, tests/test_ci_change_areas.py 49 of 49.
|
Closing this in favor of #8721, which was built on this branch and carried 20 of its commits. The commits this branch gained afterwards are now on #8721's branch as well. They cover the New Workspace failure dialog showing a localized sentence instead of raw tmux output, in all 20 locales, and the pane-seed and diagnostics routing through the session source. They also cover the reconnect-ready fan-out with its channel test, and publishing the mirror identity into each channel's own session. Every commit here has a counterpart on #8721: |
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
…alog When a routed New Workspace failed, the alert showed whatever the remote produced — tmux's stderr on the plain ssh path, or a transport error's localizedDescription. That is the implementation talking, not a message about the user's workspace, and it can carry command lines and host details the dialog has no business displaying. The alert now always shows the localized sentence: cmux could not start the session, no workspace was created, check that the host is reachable. The raw detail still reaches the debug log, which is where it is useful when someone is actually diagnosing a failure. (cherry picked from commit 59ac67c) Conflict resolved while folding manaflow-ai#8428 into this branch: this branch already suppresses the alert under a test host with a guard at the top of presentNewSessionFailureAlert. That guard stays first, and this commit's debug log of the raw detail follows it, so the dialog shows only the localized sentence.
… locale The two New Workspace dialog strings shipped with only en and ja, and the string catalog carries 20 locales. Both keys now have all 20. Two socket error strings that production already returns, workspace_id is required and the invalid session name message, had no catalog entry at all — they were falling back to their English defaultValue in every language. Both are now in the catalog with all 20 locales, and common.ok picked up the Khmer entry it was missing. Translations follow the wording already used for these terms elsewhere in the catalog, so "tmux session" reads as tmux-Sitzung in German, tmux 工作階段 in Traditional Chinese, and stays as "tmux session" in Khmer and Thai the way the neighbouring tmux strings do. (cherry picked from commit 55bdd03) Conflict resolved while folding manaflow-ai#8428 into this branch. This branch carried the New Workspace dialog strings in 19 locales, with the older message wording, and the code now shows this commit's sentence. The catalog entries for dialog.remoteTmux.newSessionFailed.title, .message and common.ok are taken from this commit, so each has all 20 locales, Khmer included, with the new wording. Every other key is unchanged from this branch.
… protocol The session-source protocol declared repaintPaneVisibleScreen(paneId:) and seedPane(paneId:) as returning nothing, which is what the control connection did when the protocol was written. The connection now returns the id of the pane-seed transaction it started (nil when none started) and takes clearScrollback, so it no longer satisfied its own protocol and the conformance stopped compiling. Both files merged cleanly on rebase, so nothing pointed at it. The id matters to a caller that reaches the source through the protocol. RemoteTmuxSessionMirror re-defers a full pane reseed when seedPane returns nil, and a source that dropped the id would leave that pane blank with nothing left to retry. So the protocol returns the id, and RemoteTmuxSessionChannel hands back the shared stream's own id unchanged: the stream owns the capture boundary and the seed bookkeeping, and a minted id would correlate with nothing. The two first-mount call sites now name clearScrollback: true, the value the connection's default already gave them, because a protocol requirement cannot carry a default. This covers the protocol surface only. The pane-seed ordering work (manaflow-ai#8436) also left the mirror calling record and beginReconnecting on the source and registering an onPaneSeed callback the source's observer bundle does not have. Those need a decision about what a shared multiplexed stream should do for one session, so they are not in here. (cherry picked from commit 2db5741) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already declares repaintPaneVisibleScreen and seedPane(paneId:clearScrollback:) returning the pane-seed transaction id, with the channel forwarding both, so the behaviour is unchanged. What this commit adds is kept: the doc comments on what the id means and why clearScrollback has no default, the channel comment on returning the shared stream's id unchanged, and the explicit clearScrollback: true at the window-mirror call site. The declarations keep this branch's shape, including beginReconnecting(preservingBackoff:).
The mirror's output routing still talked to a concrete control connection after the pane-seed path moved behind `RemoteTmuxSessionSource`, so three uses no longer resolved, and one of them had gone silently missing. `RemoteTmuxSessionObservers` had no `onPaneSeed`, so the mirror's registration dropped it and `routeSeed(paneId:seed:)` never fired. A pane mounted mid-session, or re-mounted after a reconnect, got no authoritative snapshot at all. The bundle now carries the callback, the connection conformance forwards it, and the channel fans it out behind the same `ownsPane` test as `%output`, since a seed carries a pane's whole screen and delivering one to a mirror that does not own the pane would paint another session's content into it. `record` becomes a source requirement rather than a connection-only method. Its value is that consumer and transport events land in one ordered ring that `remote.tmux.state` reads back, and following a seed failure means crossing between the two, so splitting the log would lose the ordering that makes it readable. A channel forwards to the shared stream and tags the event with its tmux session id, which stays put across renames, so one buffer written by several sessions is still attributable. `beginReconnecting()` is dropped from the mirror instead of being added to the protocol. These byte budgets are the mirror's own retention accounting, so an overflow says nothing about the health of the control stream, and on a host whose sessions share one stream, restarting it would freeze every other session's mirror over one pane's budget. All three over-budget sites now take the same repair the file's softer limits already take, dropping the pane's retained bytes and recapturing that pane with a full-history seed once its surface reaches its published grid. Each site records its own event name, so the ring still says which limit tripped. What this gives up is the guarantee that a pathological pane eventually forces a fresh control client; a stream that is genuinely unusable still reconnects from the transport's own budget and boundary guards. (cherry picked from commit 4622271) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already routes pane seeds through RemoteTmuxSessionObservers with the channel filtering them by pane ownership, and already declares and implements record(_ event:), so the behaviour is unchanged. Kept from this commit: the deferFullPaneReseed doc comment, the sentence on a pane that never receives a seed, and the explanation of why consumer events share the transport's ring, folded into this branch's record doc comment. The observers initializer keeps this branch's required members, including onAuthRequired, rather than this commit's nil defaults. The clean part of the merge had added a second record requirement and a second channel implementation that tags events session=$id, and both are removed, because this branch tags channel events [$id] and a type cannot declare the method twice.
Red half of the two-commit regression pair. The channel test drives a fake RemoteTmuxSessionSource and asserts that reconnect-ready fired on the shared stream reaches a channel observer, and stops after detach(). Mirrors schedule their post-reconnect force-resize from this event, and it is host-global — no pane or window id scopes it to one session. (cherry picked from commit 4152fbd) Conflict resolved while folding manaflow-ai#8428 into this branch: in the cmuxTests group this branch had already added RemoteTmuxRawQueryOutcomeTests.swift where this commit adds RemoteTmuxSessionChannelTests.swift, and both entries are kept.
After the shared stream reconnects and drains its seed batch, the control connection fires reconnect-ready and every mirror schedules its force-resize from it. The session channel forwarded every event except this one, so on a multiplexed host no mirror heard it and every window kept its pre-reconnect size until something else repainted it. Reconnect readiness is host-global — it carries no pane or window id to scope it to one session — so the channel fans it to all of its observers, and detach() drops it with the rest. (cherry picked from commit be4a246) Conflict resolved while folding manaflow-ai#8428 into this branch. This branch already fans onReconnectReady out through the channel and already declares and forwards sendNewPane, with the same code. The closure keeps this branch's comment, and what this commit adds is the sendNewPane stub on the test fake, so the suite's fake conforms to the protocol.
(cherry picked from commit ec33848) Conflict resolved while folding manaflow-ai#8428 into this branch. This commit's side still carried the record(_ event:) requirement that the pane-seed routing pick had already moved to the top of the protocol, so only the setMirrorEnvironment requirement is added here. The channel implementation and the republish on reconnect applied cleanly.
…branch Two follow-ups the cherry-picked manaflow-ai#8428 commits needed on this branch. The controller pushed the mirror identity with `(connection as? RemoteTmuxControlConnection)?.setMirrorEnvironment`, which skipped every multiplexed channel. setMirrorEnvironment is now a RemoteTmuxSessionSource requirement and the channel publishes to its own real session, so the call goes through the protocol and a multiplexed mirror's shell can find its local mirror too. The session-channel test fake did not conform to this branch's protocol. It implemented beginReconnecting() where the requirement is beginReconnecting(preservingBackoff:), and it had no resumeAfterInteractiveAuth(). The reconnect-ready test also built RemoteTmuxSessionObservers with one argument, while this branch's initializer requires every member. The fake now matches the protocol and the test names all eleven members.
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
…alog When a routed New Workspace failed, the alert showed whatever the remote produced — tmux's stderr on the plain ssh path, or a transport error's localizedDescription. That is the implementation talking, not a message about the user's workspace, and it can carry command lines and host details the dialog has no business displaying. The alert now always shows the localized sentence: cmux could not start the session, no workspace was created, check that the host is reachable. The raw detail still reaches the debug log, which is where it is useful when someone is actually diagnosing a failure. (cherry picked from commit 59ac67c) Conflict resolved while folding manaflow-ai#8428 into this branch: this branch already suppresses the alert under a test host with a guard at the top of presentNewSessionFailureAlert. That guard stays first, and this commit's debug log of the raw detail follows it, so the dialog shows only the localized sentence.
… locale The two New Workspace dialog strings shipped with only en and ja, and the string catalog carries 20 locales. Both keys now have all 20. Two socket error strings that production already returns, workspace_id is required and the invalid session name message, had no catalog entry at all — they were falling back to their English defaultValue in every language. Both are now in the catalog with all 20 locales, and common.ok picked up the Khmer entry it was missing. Translations follow the wording already used for these terms elsewhere in the catalog, so "tmux session" reads as tmux-Sitzung in German, tmux 工作階段 in Traditional Chinese, and stays as "tmux session" in Khmer and Thai the way the neighbouring tmux strings do. (cherry picked from commit 55bdd03) Conflict resolved while folding manaflow-ai#8428 into this branch. This branch carried the New Workspace dialog strings in 19 locales, with the older message wording, and the code now shows this commit's sentence. The catalog entries for dialog.remoteTmux.newSessionFailed.title, .message and common.ok are taken from this commit, so each has all 20 locales, Khmer included, with the new wording. Every other key is unchanged from this branch.
… protocol The session-source protocol declared repaintPaneVisibleScreen(paneId:) and seedPane(paneId:) as returning nothing, which is what the control connection did when the protocol was written. The connection now returns the id of the pane-seed transaction it started (nil when none started) and takes clearScrollback, so it no longer satisfied its own protocol and the conformance stopped compiling. Both files merged cleanly on rebase, so nothing pointed at it. The id matters to a caller that reaches the source through the protocol. RemoteTmuxSessionMirror re-defers a full pane reseed when seedPane returns nil, and a source that dropped the id would leave that pane blank with nothing left to retry. So the protocol returns the id, and RemoteTmuxSessionChannel hands back the shared stream's own id unchanged: the stream owns the capture boundary and the seed bookkeeping, and a minted id would correlate with nothing. The two first-mount call sites now name clearScrollback: true, the value the connection's default already gave them, because a protocol requirement cannot carry a default. This covers the protocol surface only. The pane-seed ordering work (manaflow-ai#8436) also left the mirror calling record and beginReconnecting on the source and registering an onPaneSeed callback the source's observer bundle does not have. Those need a decision about what a shared multiplexed stream should do for one session, so they are not in here. (cherry picked from commit 2db5741) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already declares repaintPaneVisibleScreen and seedPane(paneId:clearScrollback:) returning the pane-seed transaction id, with the channel forwarding both, so the behaviour is unchanged. What this commit adds is kept: the doc comments on what the id means and why clearScrollback has no default, the channel comment on returning the shared stream's id unchanged, and the explicit clearScrollback: true at the window-mirror call site. The declarations keep this branch's shape, including beginReconnecting(preservingBackoff:).
The mirror's output routing still talked to a concrete control connection after the pane-seed path moved behind `RemoteTmuxSessionSource`, so three uses no longer resolved, and one of them had gone silently missing. `RemoteTmuxSessionObservers` had no `onPaneSeed`, so the mirror's registration dropped it and `routeSeed(paneId:seed:)` never fired. A pane mounted mid-session, or re-mounted after a reconnect, got no authoritative snapshot at all. The bundle now carries the callback, the connection conformance forwards it, and the channel fans it out behind the same `ownsPane` test as `%output`, since a seed carries a pane's whole screen and delivering one to a mirror that does not own the pane would paint another session's content into it. `record` becomes a source requirement rather than a connection-only method. Its value is that consumer and transport events land in one ordered ring that `remote.tmux.state` reads back, and following a seed failure means crossing between the two, so splitting the log would lose the ordering that makes it readable. A channel forwards to the shared stream and tags the event with its tmux session id, which stays put across renames, so one buffer written by several sessions is still attributable. `beginReconnecting()` is dropped from the mirror instead of being added to the protocol. These byte budgets are the mirror's own retention accounting, so an overflow says nothing about the health of the control stream, and on a host whose sessions share one stream, restarting it would freeze every other session's mirror over one pane's budget. All three over-budget sites now take the same repair the file's softer limits already take, dropping the pane's retained bytes and recapturing that pane with a full-history seed once its surface reaches its published grid. Each site records its own event name, so the ring still says which limit tripped. What this gives up is the guarantee that a pathological pane eventually forces a fresh control client; a stream that is genuinely unusable still reconnects from the transport's own budget and boundary guards. (cherry picked from commit 4622271) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already routes pane seeds through RemoteTmuxSessionObservers with the channel filtering them by pane ownership, and already declares and implements record(_ event:), so the behaviour is unchanged. Kept from this commit: the deferFullPaneReseed doc comment, the sentence on a pane that never receives a seed, and the explanation of why consumer events share the transport's ring, folded into this branch's record doc comment. The observers initializer keeps this branch's required members, including onAuthRequired, rather than this commit's nil defaults. The clean part of the merge had added a second record requirement and a second channel implementation that tags events session=$id, and both are removed, because this branch tags channel events [$id] and a type cannot declare the method twice.
Red half of the two-commit regression pair. The channel test drives a fake RemoteTmuxSessionSource and asserts that reconnect-ready fired on the shared stream reaches a channel observer, and stops after detach(). Mirrors schedule their post-reconnect force-resize from this event, and it is host-global — no pane or window id scopes it to one session. (cherry picked from commit 4152fbd) Conflict resolved while folding manaflow-ai#8428 into this branch: in the cmuxTests group this branch had already added RemoteTmuxRawQueryOutcomeTests.swift where this commit adds RemoteTmuxSessionChannelTests.swift, and both entries are kept.
After the shared stream reconnects and drains its seed batch, the control connection fires reconnect-ready and every mirror schedules its force-resize from it. The session channel forwarded every event except this one, so on a multiplexed host no mirror heard it and every window kept its pre-reconnect size until something else repainted it. Reconnect readiness is host-global — it carries no pane or window id to scope it to one session — so the channel fans it to all of its observers, and detach() drops it with the rest. (cherry picked from commit be4a246) Conflict resolved while folding manaflow-ai#8428 into this branch. This branch already fans onReconnectReady out through the channel and already declares and forwards sendNewPane, with the same code. The closure keeps this branch's comment, and what this commit adds is the sendNewPane stub on the test fake, so the suite's fake conforms to the protocol.
(cherry picked from commit ec33848) Conflict resolved while folding manaflow-ai#8428 into this branch. This commit's side still carried the record(_ event:) requirement that the pane-seed routing pick had already moved to the top of the protocol, so only the setMirrorEnvironment requirement is added here. The channel implementation and the republish on reconnect applied cleanly.
…branch Two follow-ups the cherry-picked manaflow-ai#8428 commits needed on this branch. The controller pushed the mirror identity with `(connection as? RemoteTmuxControlConnection)?.setMirrorEnvironment`, which skipped every multiplexed channel. setMirrorEnvironment is now a RemoteTmuxSessionSource requirement and the channel publishes to its own real session, so the call goes through the protocol and a multiplexed mirror's shell can find its local mirror too. The session-channel test fake did not conform to this branch's protocol. It implemented beginReconnecting() where the requirement is beginReconnecting(preservingBackoff:), and it had no resumeAfterInteractiveAuth(). The reconnect-ready test also built RemoteTmuxSessionObservers with one argument, while this branch's initializer requires every member. The fake now matches the protocol and the test names all eleven members.
Fixes from the CodeRabbit/Greptile pass on manaflow-ai#8428, all in the multiplexer's own code: - Dedicated session create: a THROWN transport/timeout error from `runTmux` may have reached tmux before it surfaced, so mapping it to `.createFailed` invited a duplicate-creating retry. Report `.createIndeterminate` for the thrown case (matching the multiplexed branch) and reserve `.createFailed` for a returned, confirmed command failure. - New-window path: route both `handleMirrorNewTabRequested` overloads through one `routeMirrorNewWindow` helper. The pane-targeted overload previously used the GA bare-target builder, which in multiplexer mode resolves against the ATTACHED view session and could create the tab in the hidden view. Both now use the session-scoped builder when multiplexed. - Settlement probe: consult a multiplexer channel's underlying shared connection for pending sizing work instead of defaulting to ready, and fail closed for a source the GA harness does not recognize. - Socket error copy: describe create failures in product terms instead of leaking internal stream/subcommand vocabulary, keeping the actionable "list this host's sessions before retrying" guidance. - Leak probe: key dedup on the mirror's LIVE identity so a new mirror born at a reused address still gets a probe.
…alog When a routed New Workspace failed, the alert showed whatever the remote produced — tmux's stderr on the plain ssh path, or a transport error's localizedDescription. That is the implementation talking, not a message about the user's workspace, and it can carry command lines and host details the dialog has no business displaying. The alert now always shows the localized sentence: cmux could not start the session, no workspace was created, check that the host is reachable. The raw detail still reaches the debug log, which is where it is useful when someone is actually diagnosing a failure. (cherry picked from commit 59ac67c) Conflict resolved while folding manaflow-ai#8428 into this branch: this branch already suppresses the alert under a test host with a guard at the top of presentNewSessionFailureAlert. That guard stays first, and this commit's debug log of the raw detail follows it, so the dialog shows only the localized sentence.
… locale The two New Workspace dialog strings shipped with only en and ja, and the string catalog carries 20 locales. Both keys now have all 20. Two socket error strings that production already returns, workspace_id is required and the invalid session name message, had no catalog entry at all — they were falling back to their English defaultValue in every language. Both are now in the catalog with all 20 locales, and common.ok picked up the Khmer entry it was missing. Translations follow the wording already used for these terms elsewhere in the catalog, so "tmux session" reads as tmux-Sitzung in German, tmux 工作階段 in Traditional Chinese, and stays as "tmux session" in Khmer and Thai the way the neighbouring tmux strings do. (cherry picked from commit 55bdd03) Conflict resolved while folding manaflow-ai#8428 into this branch. This branch carried the New Workspace dialog strings in 19 locales, with the older message wording, and the code now shows this commit's sentence. The catalog entries for dialog.remoteTmux.newSessionFailed.title, .message and common.ok are taken from this commit, so each has all 20 locales, Khmer included, with the new wording. Every other key is unchanged from this branch.
… protocol The session-source protocol declared repaintPaneVisibleScreen(paneId:) and seedPane(paneId:) as returning nothing, which is what the control connection did when the protocol was written. The connection now returns the id of the pane-seed transaction it started (nil when none started) and takes clearScrollback, so it no longer satisfied its own protocol and the conformance stopped compiling. Both files merged cleanly on rebase, so nothing pointed at it. The id matters to a caller that reaches the source through the protocol. RemoteTmuxSessionMirror re-defers a full pane reseed when seedPane returns nil, and a source that dropped the id would leave that pane blank with nothing left to retry. So the protocol returns the id, and RemoteTmuxSessionChannel hands back the shared stream's own id unchanged: the stream owns the capture boundary and the seed bookkeeping, and a minted id would correlate with nothing. The two first-mount call sites now name clearScrollback: true, the value the connection's default already gave them, because a protocol requirement cannot carry a default. This covers the protocol surface only. The pane-seed ordering work (manaflow-ai#8436) also left the mirror calling record and beginReconnecting on the source and registering an onPaneSeed callback the source's observer bundle does not have. Those need a decision about what a shared multiplexed stream should do for one session, so they are not in here. (cherry picked from commit 2db5741) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already declares repaintPaneVisibleScreen and seedPane(paneId:clearScrollback:) returning the pane-seed transaction id, with the channel forwarding both, so the behaviour is unchanged. What this commit adds is kept: the doc comments on what the id means and why clearScrollback has no default, the channel comment on returning the shared stream's id unchanged, and the explicit clearScrollback: true at the window-mirror call site. The declarations keep this branch's shape, including beginReconnecting(preservingBackoff:).
The mirror's output routing still talked to a concrete control connection after the pane-seed path moved behind `RemoteTmuxSessionSource`, so three uses no longer resolved, and one of them had gone silently missing. `RemoteTmuxSessionObservers` had no `onPaneSeed`, so the mirror's registration dropped it and `routeSeed(paneId:seed:)` never fired. A pane mounted mid-session, or re-mounted after a reconnect, got no authoritative snapshot at all. The bundle now carries the callback, the connection conformance forwards it, and the channel fans it out behind the same `ownsPane` test as `%output`, since a seed carries a pane's whole screen and delivering one to a mirror that does not own the pane would paint another session's content into it. `record` becomes a source requirement rather than a connection-only method. Its value is that consumer and transport events land in one ordered ring that `remote.tmux.state` reads back, and following a seed failure means crossing between the two, so splitting the log would lose the ordering that makes it readable. A channel forwards to the shared stream and tags the event with its tmux session id, which stays put across renames, so one buffer written by several sessions is still attributable. `beginReconnecting()` is dropped from the mirror instead of being added to the protocol. These byte budgets are the mirror's own retention accounting, so an overflow says nothing about the health of the control stream, and on a host whose sessions share one stream, restarting it would freeze every other session's mirror over one pane's budget. All three over-budget sites now take the same repair the file's softer limits already take, dropping the pane's retained bytes and recapturing that pane with a full-history seed once its surface reaches its published grid. Each site records its own event name, so the ring still says which limit tripped. What this gives up is the guarantee that a pathological pane eventually forces a fresh control client; a stream that is genuinely unusable still reconnects from the transport's own budget and boundary guards. (cherry picked from commit 4622271) Conflicts resolved while folding manaflow-ai#8428 into this branch. This branch already routes pane seeds through RemoteTmuxSessionObservers with the channel filtering them by pane ownership, and already declares and implements record(_ event:), so the behaviour is unchanged. Kept from this commit: the deferFullPaneReseed doc comment, the sentence on a pane that never receives a seed, and the explanation of why consumer events share the transport's ring, folded into this branch's record doc comment. The observers initializer keeps this branch's required members, including onAuthRequired, rather than this commit's nil defaults. The clean part of the merge had added a second record requirement and a second channel implementation that tags events session=$id, and both are removed, because this branch tags channel events [$id] and a type cannot declare the method twice.
Red half of the two-commit regression pair. The channel test drives a fake RemoteTmuxSessionSource and asserts that reconnect-ready fired on the shared stream reaches a channel observer, and stops after detach(). Mirrors schedule their post-reconnect force-resize from this event, and it is host-global — no pane or window id scopes it to one session. (cherry picked from commit 4152fbd) Conflict resolved while folding manaflow-ai#8428 into this branch: in the cmuxTests group this branch had already added RemoteTmuxRawQueryOutcomeTests.swift where this commit adds RemoteTmuxSessionChannelTests.swift, and both entries are kept.
What this adds
A multiplexer transport for remote tmux: aggregate all of a host's sessions through one shared
tmux -CCview connection instead of one connection per session. This is what lets a host that permits only a single concurrent SSH connection (MaxSessions 1) mirror every session — each rendered by the exact sameRemoteTmuxSessionMirrorpath a dedicated connection uses, so there is no mirror-side special-casing. Off by default behindremoteTmux.multiplexer.beta.How it's built (reviewable in order)
RemoteTmuxSessionSource— the per-session protocol a mirror consumes; the GARemoteTmuxControlConnectionconforms directly (one connection = one session), andRemoteTmuxSessionChannelis a decorator that scopes the shared view stream to one session's windows/panes. The mirror consumesany RemoteTmuxSessionSource, so it renders both transports identically.RemoteTmuxViewConnection/RemoteTmuxViewSession/RemoteTmuxLinkedViewPlan— the hiddencmux-view-*session and its link plan.RemoteTmuxMultiplexReconciler/RemoteTmuxViewReconciler— pure session→workspace regrouping and intent-following by stable session id.RemoteTmuxController+Multiplexer+isMultiplexedbranches threaded through every teardown/reorder/new-tab/quit path.queryWithTimeouton a.rawQuerykind, thecmux_sessionssession-digest subscription +isSharedViewStream, sharedRemoteTmuxHost.fnv1a64.new-remote-workspaceprimitive (remote.tmux.new_workspace) + Cmd-N routing on a mirror workspace.Review
CodeRabbit/Greptile findings are addressed and their threads resolved:
.createIndeterminateon a thrown transport/timeout error (so a retry can't create a duplicate);.createFailedis reserved for a returned command failure;handleMirrorNewTabRequestedoverloads route through onerouteMirrorNewWindowhelper — the pane-targeted one no longer uses the GA bare-target builder, which in multiplexer mode could create the tab in the hidden view session;(The origin-colors sidebar tint that previously rode along on this branch is split out to its own PR, #7193.)
Verification
cmux-unit).RemoteTmuxMultiplexFuzzTests, 8 seeds × 250 steps): bidirectional churn — tmux-driven (add/remove/rename sessions, add/remove/shuffle windows, id-publication delay, name reuse/swaps) and cmux-driven (close/detach, pending-kill, new-remote-workspace, walk-across-sessions). It caught a real teardown bug (mirrors re-created after close because no kill intent was recorded), fixed here, and now also asserts no mirror/channel leaks after teardown.MaxSessions 1tmux host throughremote-tmux-live-fuzz.sh, 3 seeds × 20 iterations of splits / kills / window-switches / resizes: 0 failures, every pane settling to the tmux-assigned grid with per-pane text equal tocapture-pane.Review follow-up
The New Workspace failure alert used to show whatever the remote produced — tmux's stderr on the plain ssh path, or a transport error's
localizedDescription. It now always shows a localized sentence saying cmux could not start the session, that no workspace was created, and to check that the host is reachable. The raw detail still goes to the debug log, which is where it helps when someone is diagnosing the failure.The two new dialog strings shipped with only
enandja, and the string catalog carries 20 locales. Both keys now have all 20. While checking that, two socket error strings that production already returns —workspace_id is requiredand the invalid session name message — turned out to have no catalog entry at all, so they were falling back to their EnglishdefaultValuein every language; both are in the catalog now with all 20 locales, andcommon.okpicked up the Khmer entry it was missing. Translations follow the wording the catalog already uses for these terms, so a tmux session reads astmux-Sitzungin German andtmux 工作階段in Traditional Chinese, and stays astmux sessionin Khmer and Thai the way the neighbouring tmux strings do.These two commits were verified statically: the locale coverage is machine-checked against the catalog and no user-facing literal bypasses
String(localized:). Neither was compiled, since the branch does not currently apply to main.A reconnect fix that fell out of testing the shared connection: after the shared stream reconnects and drains its seed batch, the control connection fires reconnect-ready and each mirror schedules its post-reconnect force-resize from it. The session channel forwarded every event except that one, so on a multiplexed host no mirror resized after a reconnect. The channel now fans it to all of its observers — it is host-global, with no pane or window id to scope it — and the red/green pair pins it with a channel test against a fake session source.
Summary by CodeRabbit