Repository navigation
fix: restore closed remote tmux tabs by stable identity - #9108
Arthur-CWW wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughChangesRemote tmux mirrors now create in-memory closed-history records, use stable session IDs for connection reuse and reconnection, and restore through asynchronous attempt outcomes. Window-close recording avoids duplicates, while tests cover detachment, reattachment, history ordering, and restored workspace state. Remote tmux history
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TabManager
participant ClosedItemHistoryStore
participant AppDelegate
participant RemoteTmuxController
participant RemoteTmuxControlConnection
TabManager->>ClosedItemHistoryStore: pushRemoteTmuxMirror
AppDelegate->>ClosedItemHistoryStore: attemptRestore
ClosedItemHistoryStore-->>AppDelegate: pending
AppDelegate->>RemoteTmuxController: restoreClosedMirror
RemoteTmuxController->>RemoteTmuxControlConnection: attach using initialSessionId
RemoteTmuxControlConnection-->>RemoteTmuxController: connection result
RemoteTmuxController-->>AppDelegate: restore result
AppDelegate->>ClosedItemHistoryStore: completePendingRestore
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Sources/TabManager+NonInteractiveClose.swift (1)
31-39: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHistory is recorded before the close can still fail.
The mirror is detached and pushed to closed-item history, then
closeMainWindowcan returnfalseand abort. That leaves the workspace open locally (now a dead mirror) and a Recently Closed record for the same session; reopening it re-attaches and creates a second mirror workspace next to the orphan.Moving the push after the
closeMainWindowguard keeps history consistent with what actually closed.🐛 Proposed fix
if workspace.isRemoteTmuxMirror { appDelegate.remoteTmuxController.detachMirrorWorkspaceKeptOpenLocally(workspaceId: workspace.id) - if let remoteHistoryEntry { - ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) - } } guard appDelegate.closeMainWindow(windowId: windowId, recordHistory: recordHistory) else { return false } + if let remoteHistoryEntry { + ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) + }🤖 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/TabManager`+NonInteractiveClose.swift around lines 31 - 39, Move the remote mirror history push from before the closeMainWindow guard to after it succeeds, while keeping detach behavior unchanged. In the workspace.isRemoteTmuxMirror branch, defer ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) until closeMainWindow returns true so failed closes do not create Recently Closed records.Sources/RemoteTmuxController.swift (1)
1068-1079: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
stableConnectionKeywas inserted betweenconnectionKey's doc comment andconnectionKey.The comment block at 1068-1072 documents
connectionKey(host:sessionName:)but now attaches tostableConnectionKey. Move the new function belowconnectionKey, or give it its own doc comment explaining the\u{1}\u{0}id:discriminator (a tmux session name can't contain NUL, which is what keeps the two key spaces disjoint — worth stating).🤖 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/RemoteTmuxController.swift` around lines 1068 - 1079, Correct the documentation attachment around stableConnectionKey and connectionKey: move stableConnectionKey below connectionKey so the existing comment remains attached to connectionKey, or add a dedicated comment for stableConnectionKey explaining its NUL-based id discriminator and disjoint key space.Sources/RemoteTmuxControlConnection.swift (1)
342-401: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAn unbounded restore wait makes the pending-record state unbounded too.
waitUntilConnected(allowingReconnect: false)resolves only on a connection state transition or task cancellation, and the restore caller offers neither a deadline nor a cancellation path. A connection that authenticates but never reaches control mode keeps the history record inpendingRestoreRecordIdsforever, and the new pending guard inremoveRecord(id:)then turns any explicit removal of that record into a silent no-op.
Sources/RemoteTmuxControlConnection.swift#L342-L401: bound the initial-attempt wait with a real deadline owned by the restore operation (not aTask.sleeppolling loop), so the waiter always resolves.Sources/ClosedItemHistory.swift#L337-L346: make removal of a pending record deterministic — either clear the pending id and remove, or return a distinct result so callers can tell "pending" from "not found".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/RemoteTmuxControlConnection.swift` around lines 342 - 401, Bound the initial-attempt wait in waitUntilConnected(allowingReconnect:) with a real deadline owned by the restore operation, ensuring the waiter always resolves without polling. In Sources/RemoteTmuxControlConnection.swift lines 342-401, preserve immediate state handling and cancellation while adding deadline-driven completion for the non-reconnecting path; in Sources/ClosedItemHistory.swift lines 337-346, update removeRecord(id:) so pending records are deterministically cleared and removed, or return a distinct pending result that callers can handle.Sources/ClosedItemHistory.swift (1)
337-346: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
removeRecord(id:)now silently fails for pending ids.A record whose async restore is in flight can no longer be removed; callers get
nilwith no distinction from "not found".TabManager.reopenClosedHistoryItem(non-AppDelegate path) then just returnsfalse, and any explicit menu removal is a silent no-op until the pending attach resolves. SincewaitUntilConnectedhas no explicit timeout, that window is bounded only by ssh keepalives.Consider either letting removal win (drop the pending id and remove) or surfacing the pending state to the caller so the UI can reflect it.
🤖 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/ClosedItemHistory.swift` around lines 337 - 346, Update removeRecord(id:) so pending restore IDs do not block removal: remove the ID from pendingRestoreRecordIds before locating and removing the matching record, allowing explicit removal to win over an in-flight restore. Preserve the existing revision increment, persistence, and returned record/index behavior.
🤖 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/RemoteTmuxMirrorCloseDetachTests.swift`:
- Around line 56-57: Both test setup blocks in
cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift at lines 56-57 and 313-314 must
stop calling ClosedItemHistoryStore.shared.removeAll() for isolation. Snapshot
the existing records before each test and restore them in defer, or inject a
test-scoped ClosedItemHistoryStore, preserving any pre-existing persisted
history at both sites.
- Line 367: Ensure the panel-count baseline in RemoteTmuxMirrorCloseDetachTests
is nonzero before capturing closedLocalPanelCount, since autoWelcomeIfNeeded:
false can create a workspace with no panels and make the restore assertion
vacuous. Update the setup around localWorkspace and the assertion near
closedLocalPanelCount so the test creates or validates at least one panel while
preserving the existing panel-count comparison.
In `@Sources/AppDelegate.swift`:
- Around line 14433-14439: Align activation behavior between the reopen shortcut
and the Recently Closed menu entrypoint: update reopenClosedHistoryItem or its
menu call site so both explicitly use the chosen shouldActivate policy. Keep the
shared reopenMostRecentlyClosedItem path consistent and make any intentional
difference explicit at the caller.
- Around line 16588-16620: Reverse the history insertion order in the
window-close flow: push each remote mirror entry via pushRemoteTmuxMirror before
pushing the local window record via ClosedItemHistoryStore.shared.push. Preserve
the existing workspace-order iteration and window snapshot creation, so the
window is restored first and subsequent mirror restoration can resolve its
recorded windowId and workspaceIndex.
In `@Sources/AppDelegate`+ClosedItemHistory.swift:
- Line 15: Restore the default value of shouldActivate in the history-restore
API to true, and ensure the menu and palette command call sites in
cmuxApp+HistoryMenu and ContentView pass true when invoking restores. Preserve
non-activating behavior only for callers that explicitly request false.
- Around line 81-95: The pending restore path around restoreClosedMirror must
use a cancellable, record-scoped task instead of an untracked Task. Add storage
in the owning controller or manager keyed by record.id, cancel and replace any
existing task for that record, and remove it when the attempt finishes or is
cancelled; only then call completePendingRestore and beep for the current
attempt.
In `@Sources/ClosedItemHistory.swift`:
- Around line 827-833: Update the .remoteTmuxMirror case in
ClosedItemHistoryMenuItem to use a dedicated localized key and label for remote
tmux mirrors instead of menu.history.recentlyClosed.kind.workspace. Add the
corresponding key to Resources/Localizable.xcstrings with translations for every
supported locale.
- Around line 452-462: Introduce a named static capacity constant on the
containing type for the remote tmux mirror limit, then replace the literal 20 in
trimRemoteTmuxMirrorRecordsIfNeeded() with that constant. Keep the existing
trimming behavior unchanged.
In `@Sources/RemoteTmuxControlConnection.swift`:
- Around line 330-341: Rename the parameter of
RemoteTmuxHost.controlModeArguments from sessionName to target and update its
documentation and internal references, preserving the existing single-quoting
behavior for both session names and $id targets. Update all callers, including
RemoteTmuxControlConnection.start, to use the new label.
In `@Sources/RemoteTmuxController.swift`:
- Around line 176-182: Remove the unused host and sessionName parameters from
stopCachedConnectionIfCurrent and update its sole call site accordingly, or
inline removeCachedConnection(connection)?.stop() at that call site; eliminate
the misleading one-line wrapper.
- Around line 518-520: Update the catch block on the restore/re-attach path to
log the failure before returning false, following the existing logging pattern
in RemoteTmuxController that records host.connectionHash with privacy set to
public rather than logging the destination. Preserve the current false return
behavior.
In `@Sources/TabManager.swift`:
- Around line 2108-2121: Combine the consecutive workspace.isRemoteTmuxMirror
checks in the surrounding tab-closing logic into one conditional, keeping both
detachMirrorWorkspaceKeptOpenLocally and pushRemoteTmuxMirror operations within
that branch; preserve the existing else-if history behavior for non-mirror
workspaces.
---
Outside diff comments:
In `@Sources/ClosedItemHistory.swift`:
- Around line 337-346: Update removeRecord(id:) so pending restore IDs do not
block removal: remove the ID from pendingRestoreRecordIds before locating and
removing the matching record, allowing explicit removal to win over an in-flight
restore. Preserve the existing revision increment, persistence, and returned
record/index behavior.
In `@Sources/RemoteTmuxControlConnection.swift`:
- Around line 342-401: Bound the initial-attempt wait in
waitUntilConnected(allowingReconnect:) with a real deadline owned by the restore
operation, ensuring the waiter always resolves without polling. In
Sources/RemoteTmuxControlConnection.swift lines 342-401, preserve immediate
state handling and cancellation while adding deadline-driven completion for the
non-reconnecting path; in Sources/ClosedItemHistory.swift lines 337-346, update
removeRecord(id:) so pending records are deterministically cleared and removed,
or return a distinct pending result that callers can handle.
In `@Sources/RemoteTmuxController.swift`:
- Around line 1068-1079: Correct the documentation attachment around
stableConnectionKey and connectionKey: move stableConnectionKey below
connectionKey so the existing comment remains attached to connectionKey, or add
a dedicated comment for stableConnectionKey explaining its NUL-based id
discriminator and disjoint key space.
In `@Sources/TabManager`+NonInteractiveClose.swift:
- Around line 31-39: Move the remote mirror history push from before the
closeMainWindow guard to after it succeeds, while keeping detach behavior
unchanged. In the workspace.isRemoteTmuxMirror branch, defer
ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) until
closeMainWindow returns true so failed closes do not create Recently Closed
records.
🪄 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 Plus
Run ID: 6251cee1-5172-4e61-a98e-0cc945a19f2b
📒 Files selected for processing (9)
Sources/AppDelegate+ClosedItemHistory.swiftSources/AppDelegate.swiftSources/ClosedItemHistory.swiftSources/RemoteTmuxControlConnection.swiftSources/RemoteTmuxController.swiftSources/RemoteTmuxHost.swiftSources/TabManager+NonInteractiveClose.swiftSources/TabManager.swiftcmuxTests/RemoteTmuxMirrorCloseDetachTests.swift
| ClosedItemHistoryStore.shared.removeAll() | ||
| defer { ClosedItemHistoryStore.shared.removeAll() } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Both new tests clear the app-wide persisted closed-item history instead of isolating it. The shared root cause is using ClosedItemHistoryStore.shared.removeAll() for test isolation, which destroys pre-existing (and locally persisted) history rather than scoping state to the test.
cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift#L56-L57: snapshot the existing records and restore them indefer, or inject a test-scoped store instead ofremoveAll().cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift#L313-L314: apply the same snapshot/restore or injected-store approach here.
📍 Affects 1 file
cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift#L56-L57(this comment)cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift#L313-L314
🤖 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 `@cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift` around lines 56 - 57, Both
test setup blocks in cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift at lines
56-57 and 313-314 must stop calling ClosedItemHistoryStore.shared.removeAll()
for isolation. Snapshot the existing records before each test and restore them
in defer, or inject a test-scoped ClosedItemHistoryStore, preserving any
pre-existing persisted history at both sites.
Source: Coding guidelines
| select: false, | ||
| autoWelcomeIfNeeded: false | ||
| ) | ||
| let closedLocalPanelCount = localWorkspace.panels.count |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Guard the panel-count baseline so the restore assertion can't pass vacuously.
localWorkspace is created with autoWelcomeIfNeeded: false; if it has zero panels, Line 446 asserts 0 == 0 and no longer proves panel preservation.
💚 Proposed fix
let closedLocalPanelCount = localWorkspace.panels.count
+ `#expect`(closedLocalPanelCount > 0)Also applies to: 446-446
🤖 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 `@cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift` at line 367, Ensure the
panel-count baseline in RemoteTmuxMirrorCloseDetachTests is nonzero before
capturing closedLocalPanelCount, since autoWelcomeIfNeeded: false can create a
workspace with no panels and make the restore assertion vacuous. Update the
setup around localWorkspace and the assertion near closedLocalPanelCount so the
test creates or validates at least one panel while preserving the existing
panel-count comparison.
| if matchConfiguredShortcut(event: event, action: .reopenClosedBrowserPanel) { | ||
| let routedManager = preferredMainWindowContextForShortcutRouting(event: event)?.tabManager ?? tabManager | ||
| _ = reopenMostRecentlyClosedItem(preferredTabManager: routedManager) | ||
| _ = reopenMostRecentlyClosedItem( | ||
| preferredTabManager: routedManager, | ||
| shouldActivate: true | ||
| ) | ||
| return true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Activation now differs between reopen entrypoints.
This shortcut passes shouldActivate: true, but the Recently Closed menu path (reopenClosedHistoryItem) still defaults to false, so clicking a menu entry restores without selecting/focusing while Cmd+Shift+T does. As per coding guidelines ("When behavior has multiple entrypoints, implement one shared action path and verify every entrypoint"), pick one activation policy and apply it to both, or make the difference explicit at the menu call site.
🤖 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/AppDelegate.swift` around lines 14433 - 14439, Align activation
behavior between the reopen shortcut and the Recently Closed menu entrypoint:
update reopenClosedHistoryItem or its menu call site so both explicitly use the
chosen shouldActivate policy. Keep the shared reopenMostRecentlyClosedItem path
consistent and make any intentional difference explicit at the caller.
Source: Coding guidelines
| // Capture remote mirrors while their controller registrations and live | ||
| // connection identities still exist. Session snapshots intentionally omit | ||
| // them, so the ordinary window snapshot alone cannot represent this close. | ||
| let remoteMirrorEntries: [ClosedRemoteTmuxMirrorHistoryEntry] = context.tabManager.tabs.enumerated().compactMap { index, workspace in | ||
| guard workspace.isRemoteTmuxMirror else { return nil } | ||
| return remoteTmuxController.closedMirrorHistoryEntry( | ||
| workspaceId: workspace.id, | ||
| windowId: context.windowId, | ||
| workspaceIndex: index | ||
| ) | ||
| } | ||
|
|
||
| let restorableAgentIndex = SharedLiveAgentIndex.shared.currentIndexSchedulingRefresh() | ||
| ?? RestorableAgentSessionIndex.load() | ||
| guard let snapshot = closeWindowSnapshotPruningCrashDiagnostics( | ||
| if let snapshot = closeWindowSnapshotPruningCrashDiagnostics( | ||
| for: context, | ||
| includeScrollback: true, | ||
| restorableAgentIndex: restorableAgentIndex | ||
| ).snapshot else { | ||
| return | ||
| ).snapshot, | ||
| !snapshot.tabManager.workspaces.isEmpty { | ||
| // Preserve the existing synchronous local-window history ordering. | ||
| ClosedItemHistoryStore.shared.push(.window(ClosedWindowHistoryEntry( | ||
| windowId: context.windowId, | ||
| snapshot: snapshot, | ||
| workspaceIds: snapshot.tabManager.workspaces.compactMap(\.workspaceId) | ||
| ))) | ||
| } | ||
| guard !snapshot.tabManager.workspaces.isEmpty else { | ||
| return | ||
|
|
||
| // Push after the local window record, in the manager's workspace order. | ||
| // Remote entries remain memory-only and retain their independent 20-record cap. | ||
| for entry in remoteMirrorEntries { | ||
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(entry) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Push order strands remote mirrors in the wrong window on a window close.
Remote entries are pushed after the local window record, so they are newest and reopen first. At that moment the recorded windowId no longer resolves — restoreTargetManager sees appDelegate.mainWindow(for: windowId) == nil and falls back to preferredTabManager/appDelegate.tabManager. The mirror re-attaches into whatever other window is open, placeRestoredMirror computes isInRecordedWindow == false so the recorded workspaceIndex is skipped too, and the next Cmd+Shift+T then recreates the original window without its mirror.
Pushing the mirror entries before the window record restores the window first, so the recorded windowId resolves on the following mirror restore and both placement and ordering land correctly.
🐛 Proposed fix: record mirrors before the window
+ // Push before the local window record so the window record is newest and
+ // reopens first; the mirror restores that follow can then resolve their
+ // recorded windowId and land back in their original window.
+ for entry in remoteMirrorEntries {
+ ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(entry)
+ }
+
let restorableAgentIndex = SharedLiveAgentIndex.shared.currentIndexSchedulingRefresh()
?? RestorableAgentSessionIndex.load()
if let snapshot = closeWindowSnapshotPruningCrashDiagnostics(
for: context,
includeScrollback: true,
restorableAgentIndex: restorableAgentIndex
).snapshot,
!snapshot.tabManager.workspaces.isEmpty {
// Preserve the existing synchronous local-window history ordering.
ClosedItemHistoryStore.shared.push(.window(ClosedWindowHistoryEntry(
windowId: context.windowId,
snapshot: snapshot,
workspaceIds: snapshot.tabManager.workspaces.compactMap(\.workspaceId)
)))
}
-
- // Push after the local window record, in the manager's workspace order.
- // Remote entries remain memory-only and retain their independent 20-record cap.
- for entry in remoteMirrorEntries {
- ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(entry)
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Capture remote mirrors while their controller registrations and live | |
| // connection identities still exist. Session snapshots intentionally omit | |
| // them, so the ordinary window snapshot alone cannot represent this close. | |
| let remoteMirrorEntries: [ClosedRemoteTmuxMirrorHistoryEntry] = context.tabManager.tabs.enumerated().compactMap { index, workspace in | |
| guard workspace.isRemoteTmuxMirror else { return nil } | |
| return remoteTmuxController.closedMirrorHistoryEntry( | |
| workspaceId: workspace.id, | |
| windowId: context.windowId, | |
| workspaceIndex: index | |
| ) | |
| } | |
| let restorableAgentIndex = SharedLiveAgentIndex.shared.currentIndexSchedulingRefresh() | |
| ?? RestorableAgentSessionIndex.load() | |
| guard let snapshot = closeWindowSnapshotPruningCrashDiagnostics( | |
| if let snapshot = closeWindowSnapshotPruningCrashDiagnostics( | |
| for: context, | |
| includeScrollback: true, | |
| restorableAgentIndex: restorableAgentIndex | |
| ).snapshot else { | |
| return | |
| ).snapshot, | |
| !snapshot.tabManager.workspaces.isEmpty { | |
| // Preserve the existing synchronous local-window history ordering. | |
| ClosedItemHistoryStore.shared.push(.window(ClosedWindowHistoryEntry( | |
| windowId: context.windowId, | |
| snapshot: snapshot, | |
| workspaceIds: snapshot.tabManager.workspaces.compactMap(\.workspaceId) | |
| ))) | |
| } | |
| guard !snapshot.tabManager.workspaces.isEmpty else { | |
| return | |
| // Push after the local window record, in the manager's workspace order. | |
| // Remote entries remain memory-only and retain their independent 20-record cap. | |
| for entry in remoteMirrorEntries { | |
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(entry) | |
| } | |
| // Capture remote mirrors while their controller registrations and live | |
| // connection identities still exist. Session snapshots intentionally omit | |
| // them, so the ordinary window snapshot alone cannot represent this close. | |
| let remoteMirrorEntries: [ClosedRemoteTmuxMirrorHistoryEntry] = context.tabManager.tabs.enumerated().compactMap { index, workspace in | |
| guard workspace.isRemoteTmuxMirror else { return nil } | |
| return remoteTmuxController.closedMirrorHistoryEntry( | |
| workspaceId: workspace.id, | |
| windowId: context.windowId, | |
| workspaceIndex: index | |
| ) | |
| } | |
| // Push before the local window record so the window record is newest and | |
| // reopens first; the mirror restores that follow can then resolve their | |
| // recorded windowId and land back in their original window. | |
| for entry in remoteMirrorEntries { | |
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(entry) | |
| } | |
| let restorableAgentIndex = SharedLiveAgentIndex.shared.currentIndexSchedulingRefresh() | |
| ?? RestorableAgentSessionIndex.load() | |
| if let snapshot = closeWindowSnapshotPruningCrashDiagnostics( | |
| for: context, | |
| includeScrollback: true, | |
| restorableAgentIndex: restorableAgentIndex | |
| ).snapshot, | |
| !snapshot.tabManager.workspaces.isEmpty { | |
| // Preserve the existing synchronous local-window history ordering. | |
| ClosedItemHistoryStore.shared.push(.window(ClosedWindowHistoryEntry( | |
| windowId: context.windowId, | |
| snapshot: snapshot, | |
| workspaceIds: snapshot.tabManager.workspaces.compactMap(\.workspaceId) | |
| ))) | |
| } |
🤖 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/AppDelegate.swift` around lines 16588 - 16620, Reverse the history
insertion order in the window-close flow: push each remote mirror entry via
pushRemoteTmuxMirror before pushing the local window record via
ClosedItemHistoryStore.shared.push. Preserve the existing workspace-order
iteration and window snapshot creation, so the window is restored first and
subsequent mirror restoration can resolve its recorded windowId and
workspaceIndex.
| func reopenMostRecentlyClosedItem( | ||
| preferredTabManager: TabManager? = nil, | ||
| shouldActivate: Bool = true | ||
| shouldActivate: Bool = false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\b(reopenMostRecentlyClosedItem|reopenClosedHistoryItem)\s*\(' --type=swiftRepository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -a 'AppDelegate\+ClosedItemHistory\.swift$' .
echo
echo "== matching references =="
rg -n -C3 'reopenMostRecentlyClosedItem|reopenClosedHistoryItem' . || true
echo
echo "== AppDelegate+ClosedItemHistory outline/contents =="
for f in $(fd 'AppDelegate\+ClosedItemHistory\.swift$' .); do
echo "--- $f"
wc -l "$f"
sed -n '1,230p' "$f" | cat -n
doneRepository: manaflow-ai/cmux
Length of output: 42221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant entrypoints =="
sed -n '32,46p;96,108p;14428,14438p;8256,8270p' Sources/AppDelegate+ClosedItemHistory.swift | cat -n
echo
echo "== all direct caller argument patterns =="
python3 - <<'PY'
import pathlib, re
for path in pathlib.Path('.').rglob('*.swift'):
if '/cmuxTests/' in str(path):
continue
text = path.read_text(errors='ignore').splitlines()
for i,line in enumerate(text,1):
if re.search(r'reopenMostRecentlyClosedItem\s*\(|reopenClosedHistoryItem\s*\(', line):
# print next 3 non-empty lines and whether shouldActivate appears
snippets=[]
base=max(1,i-6); top=min(len(text),i+8)
for j in range(base,top):
snippets.append(text[j-1])
joined='\n'.join(snippets)
if 'shouldActivate' in joined:
print(f"{path}:{i} SHOULD_ACTIVATE_PRESENT")
else:
print(f"{path}:{i} SHOULD_ACTIVATE_MISSING")
for k,s in enumerate(snippets, base):
print(f"{path}:{k}: {s}")
print("---")
PYRepository: manaflow-ai/cmux
Length of output: 50372
Keep history restores activating by default.
Production callers that rely on the default (cmuxApp+HistoryMenu.swift and ContentView.swift palette commands) now call shouldActivate = false, so menu and palette reopen commands can restore panels/workspaces without activating the owning window. Keep the default true, or pass shouldActivate: true from those UI entrypoints.
🤖 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/AppDelegate`+ClosedItemHistory.swift at line 15, Restore the default
value of shouldActivate in the history-restore API to true, and ensure the menu
and palette command call sites in cmuxApp+HistoryMenu and ContentView pass true
when invoking restores. Preserve non-activating behavior only for callers that
explicitly request false.
| case .remoteTmuxMirror(let entry): | ||
| return ClosedItemHistoryMenuItem( | ||
| id: record.id, | ||
| title: entry.sessionName, | ||
| detail: String(localized: "menu.history.recentlyClosed.kind.workspace", defaultValue: "Workspace"), | ||
| closedAt: record.closedAt | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remote mirrors render as "Workspace" in Recently Closed.
Reusing menu.history.recentlyClosed.kind.workspace makes a closed remote tmux mirror indistinguishable from a local workspace with the same title, even though reopening them does very different things (re-attach over ssh vs. restore a local snapshot). A dedicated key would disambiguate — note it needs matching entries in Resources/Localizable.xcstrings for every supported locale.
🤖 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/ClosedItemHistory.swift` around lines 827 - 833, Update the
.remoteTmuxMirror case in ClosedItemHistoryMenuItem to use a dedicated localized
key and label for remote tmux mirrors instead of
menu.history.recentlyClosed.kind.workspace. Add the corresponding key to
Resources/Localizable.xcstrings with translations for every supported locale.
Source: Coding guidelines
| /// Spawns the SSH `tmux -CC` process and begins streaming. | ||
| func start() throws { | ||
| guard !started else { return } | ||
| try host.ensureControlSocketDirectory() | ||
| // The initial connect honors `createIfMissing`; reconnects never create. | ||
| try spawnProcess(createIfMissing: createIfMissing) | ||
| // Initial connects and reconnects target the immutable tmux session id | ||
| // whenever one is known. A mutable name is only the final fallback. | ||
| let attachTarget = createIfMissing | ||
| ? sessionName | ||
| : stableSessionId.map { "$\($0)" } ?? sessionName | ||
| try spawnProcess(createIfMissing: createIfMissing, attachTarget: attachTarget) | ||
| started = true | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
controlModeArguments(sessionName:) now receives an id-bearing target.
Passing "$\(id)" through a parameter named sessionName (whose doc comment says "the tmux session to attach to (or create)") invites a future caller to shell-quote or validate it as a name. Renaming the parameter to target on RemoteTmuxHost.controlModeArguments would keep the contract honest; the single-quoting there is already correct for $id.
🤖 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/RemoteTmuxControlConnection.swift` around lines 330 - 341, Rename the
parameter of RemoteTmuxHost.controlModeArguments from sessionName to target and
update its documentation and internal references, preserving the existing
single-quoting behavior for both session names and $id targets. Update all
callers, including RemoteTmuxControlConnection.start, to use the new label.
| private func stopCachedConnectionIfCurrent( | ||
| _ connection: RemoteTmuxControlConnection, | ||
| host: RemoteTmuxHost, | ||
| sessionName: String | ||
| host _: RemoteTmuxHost, | ||
| sessionName _: String | ||
| ) { | ||
| let key = Self.connectionKey(host: host, sessionName: sessionName) | ||
| guard connectionsByHostSession[key] === connection else { return } | ||
| removeCachedConnection(forKey: key)?.stop() | ||
| removeCachedConnection(connection)?.stop() | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Dead parameters on stopCachedConnectionIfCurrent.
host and sessionName are now unused (_:), leaving a one-line wrapper with a misleading signature and a single call site. Either drop the parameters or inline removeCachedConnection(connection)?.stop() at line 169.
🤖 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/RemoteTmuxController.swift` around lines 176 - 182, Remove the unused
host and sessionName parameters from stopCachedConnectionIfCurrent and update
its sole call site accordingly, or inline
removeCachedConnection(connection)?.stop() at that call site; eliminate the
misleading one-line wrapper.
| } catch { | ||
| return false | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Silent catch on the restore path.
A failed re-attach surfaces to the user as nothing but an NSSound.beep() from the caller, with no diagnostic anywhere. This file already has the safe pattern for this (line 82 logs host.connectionHash with privacy: .public rather than the destination).
♻️ Proposed diagnostic
} catch {
+ Self.logger.warning(
+ "remote-tmux: closed-mirror restore failed [\(entry.host.connectionHash, privacy: .public)]"
+ )
return false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch { | |
| return false | |
| } | |
| } catch { | |
| Self.logger.warning( | |
| "remote-tmux: closed-mirror restore failed [\(entry.host.connectionHash, privacy: .public)]" | |
| ) | |
| return false | |
| } |
🤖 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/RemoteTmuxController.swift` around lines 518 - 520, Update the catch
block on the restore/re-attach path to log the failure before returning false,
following the existing logging pattern in RemoteTmuxController that records
host.connectionHash with privacy set to public rather than logging the
destination. Preserve the current false return behavior.
Source: Coding guidelines
| // Closing a mirrored remote tmux workspace DETACHES from the remote session, | ||
| // leaving it alive on the server for resume. Killing the session is never a | ||
| // side effect of closing a tab (PR #7264 review); it is only ever an explicit | ||
| // disconnect action. | ||
| if workspace.isRemoteTmuxMirror { | ||
| AppDelegate.shared?.remoteTmuxController.detachMirrorWorkspaceKeptOpenLocally(workspaceId: workspace.id) | ||
| } | ||
| if recordHistory, | ||
| workspace.isRestorableInSessionSnapshot, | ||
| let index = tabs.firstIndex(where: { $0.id == workspace.id }) { | ||
| if workspace.isRemoteTmuxMirror { | ||
| if let remoteHistoryEntry { | ||
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) | ||
| } | ||
| } else if recordHistory, | ||
| workspace.isRestorableInSessionSnapshot, | ||
| let index = tabs.firstIndex(where: { $0.id == workspace.id }) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Fold the two consecutive isRemoteTmuxMirror checks.
2112 and 2115 test the same condition back-to-back; the split makes the else if chain read as though the detach and the history push are independent decisions.
♻️ Proposed refactor
if workspace.isRemoteTmuxMirror {
AppDelegate.shared?.remoteTmuxController.detachMirrorWorkspaceKeptOpenLocally(workspaceId: workspace.id)
- }
- if workspace.isRemoteTmuxMirror {
if let remoteHistoryEntry {
ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry)
}
} else if recordHistory,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Closing a mirrored remote tmux workspace DETACHES from the remote session, | |
| // leaving it alive on the server for resume. Killing the session is never a | |
| // side effect of closing a tab (PR #7264 review); it is only ever an explicit | |
| // disconnect action. | |
| if workspace.isRemoteTmuxMirror { | |
| AppDelegate.shared?.remoteTmuxController.detachMirrorWorkspaceKeptOpenLocally(workspaceId: workspace.id) | |
| } | |
| if recordHistory, | |
| workspace.isRestorableInSessionSnapshot, | |
| let index = tabs.firstIndex(where: { $0.id == workspace.id }) { | |
| if workspace.isRemoteTmuxMirror { | |
| if let remoteHistoryEntry { | |
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) | |
| } | |
| } else if recordHistory, | |
| workspace.isRestorableInSessionSnapshot, | |
| let index = tabs.firstIndex(where: { $0.id == workspace.id }) { | |
| // Closing a mirrored remote tmux workspace DETACHES from the remote session, | |
| // leaving it alive on the server for resume. Killing the session is never a | |
| // side effect of closing a tab (PR `#7264` review); it is only ever an explicit | |
| // disconnect action. | |
| if workspace.isRemoteTmuxMirror { | |
| AppDelegate.shared?.remoteTmuxController.detachMirrorWorkspaceKeptOpenLocally(workspaceId: workspace.id) | |
| if let remoteHistoryEntry { | |
| ClosedItemHistoryStore.shared.pushRemoteTmuxMirror(remoteHistoryEntry) | |
| } | |
| } else if recordHistory, | |
| workspace.isRestorableInSessionSnapshot, | |
| let index = tabs.firstIndex(where: { $0.id == workspace.id }) { |
🤖 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/TabManager.swift` around lines 2108 - 2121, Combine the consecutive
workspace.isRemoteTmuxMirror checks in the surrounding tab-closing logic into
one conditional, keeping both detachMirrorWorkspaceKeptOpenLocally and
pushRemoteTmuxMirror operations within that branch; preserve the existing
else-if history behavior for non-mirror workspaces.
fa8ddb2 to
45cefd9
Compare
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Sources/RemoteTmuxController.swift (3)
101-105: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftAvoid an O(N²) scan during bulk mirroring.
mirrorSessionsattaches once per discovered session, but eachattachnow scans allconnectionsByHostSession.values. For a host with N sessions, this adds O(N²) cache work on the MainActor. Maintain a secondary index keyed by(connectionHash, stableSessionId)and update it on cache, rekey, and removal so this lookup remains O(1).As per path instructions, avoid repeated full-collection scans in production paths over scalable user data.
🤖 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/RemoteTmuxController.swift` around lines 101 - 105, Replace the full connectionsByHostSession.values scan in the attach logic with a secondary index keyed by connectionHash and stableSessionId. Maintain this index whenever connections are cached, rekeyed, or removed, and use it for the initialSessionId lookup so bulk mirrorSessions attachments remain O(1) per session.Source: Path instructions
110-119: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFail closed when restoration has a stable session ID.
When
initialSessionIdis known, this branch still reuses a name-keyed connection whose stable ID isnil. If a remote session was recreated under the same name, that connection may represent a different session. The restore path then places any mirror attached to it before revalidatingliveMirror(matching:), allowing the wrong closed item to reopen.Require the cached connection’s known or initial identity to match before reuse, and validate the mirror against
entrybefore the early placement path.As per path instructions, correctness-critical restoration must use structured IDs and fail closed when the reliable signal is missing.
Also applies to: 467-474
🤖 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/RemoteTmuxController.swift` around lines 110 - 119, Update the cached-connection reuse logic in RemoteTmuxController around connectionsByHostSession and stableSessionId to fail closed during restoration: when initialSessionId is known, reuse only a connection whose stable identity is known and matches it. In the early restore-placement path, validate liveMirror(matching:) against entry before placing any mirror, and apply the same identity and mirror checks to the corresponding logic around the referenced later branch.Source: Path instructions
121-142: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep stable-ID cache keys compatible with every lifecycle path.
When a name key is occupied by a different stable ID,
attachandcreateMirrorstore the new connection/mirror understableConnectionKey. However,detach,tearDownMirrorAndCloseWorkspace,connection(host:sessionName:), andsessionMirror(host:sessionName:)still read or remove only the name key. This can leave the stable-keyed mirror and connection alive after teardown, or return the older name-keyed session.Centralize identity-aware lookup/removal and update all lifecycle APIs before introducing the second key; name-only operations should fail closed when the identity is ambiguous.
As per path instructions, correctness-critical session identity must have one authoritative structured source rather than disagreeing name and ID maps.
Also applies to: 368-378
🤖 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/RemoteTmuxController.swift` around lines 121 - 142, Centralize session identity resolution and removal around one authoritative structured source, then update attach/createMirror cache handling and the lifecycle APIs detach, tearDownMirrorAndCloseWorkspace, connection(host:sessionName:), and sessionMirror(host:sessionName:). Ensure stable-ID keys are consistently found and removed across every path, while name-only operations fail closed when multiple stable identities make the name ambiguous.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.
Inline comments:
In `@Sources/RemoteTmuxController.swift`:
- Around line 890-897: Update the explicit-detach cleanup around
closeWorkspaceNonInteractively to use the same authoritative window ID resolved
by the close operation, failing closed when that ID is unavailable. Capture the
helper’s Boolean result and only forget that exact recoverable route when the
close succeeds and the relevant tabs are empty; do not perform route cleanup
after a failed close.
---
Outside diff comments:
In `@Sources/RemoteTmuxController.swift`:
- Around line 101-105: Replace the full connectionsByHostSession.values scan in
the attach logic with a secondary index keyed by connectionHash and
stableSessionId. Maintain this index whenever connections are cached, rekeyed,
or removed, and use it for the initialSessionId lookup so bulk mirrorSessions
attachments remain O(1) per session.
- Around line 110-119: Update the cached-connection reuse logic in
RemoteTmuxController around connectionsByHostSession and stableSessionId to fail
closed during restoration: when initialSessionId is known, reuse only a
connection whose stable identity is known and matches it. In the early
restore-placement path, validate liveMirror(matching:) against entry before
placing any mirror, and apply the same identity and mirror checks to the
corresponding logic around the referenced later branch.
- Around line 121-142: Centralize session identity resolution and removal around
one authoritative structured source, then update attach/createMirror cache
handling and the lifecycle APIs detach, tearDownMirrorAndCloseWorkspace,
connection(host:sessionName:), and sessionMirror(host:sessionName:). Ensure
stable-ID keys are consistently found and removed across every path, while
name-only operations fail closed when multiple stable identities make the name
ambiguous.
🪄 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 Plus
Run ID: b57daf1b-271e-4534-b6e5-19ef16a50ba1
📒 Files selected for processing (2)
Sources/RemoteTmuxController.swiftcmuxTests/RemoteTmuxMirrorCloseDetachTests.swift
💤 Files with no reviewable changes (1)
- cmuxTests/RemoteTmuxMirrorCloseDetachTests.swift
| // its owning window avoids stranding a blank `--new-window` shell, | ||
| // and the closed window must not linger as a recoverable route — | ||
| // recovering it would resurrect a dead remote path. | ||
| let owningWindowId = manager.windowId | ||
| _ = manager.closeWorkspaceNonInteractively(workspace, allowPinned: true) | ||
| if let owningWindowId, manager.tabs.isEmpty { | ||
| AppDelegate.shared?.forgetRecoverableMainWindowRoute(windowId: owningWindowId) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make explicit-detach cleanup conditional on a successful close.
closeWorkspaceNonInteractively can return false, but this call ignores the result after the mirror and connection have already been removed. A failed close can therefore leave a visible remote workspace with no live mirror or connection. Additionally, the close helper resolves the window through AppDelegate.windowId(for:), while this block snapshots manager.windowId; if those differ, route cleanup can miss or clear the wrong recoverable route.
Use the same authoritative window ID as the close operation, require the close to succeed, and only then forget that exact route.
As per path instructions, restoration and route decisions must use one authoritative identity source and fail closed when it is unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/RemoteTmuxController.swift` around lines 890 - 897, Update the
explicit-detach cleanup around closeWorkspaceNonInteractively to use the same
authoritative window ID resolved by the close operation, failing closed when
that ID is unavailable. Capture the helper’s Boolean result and only forget that
exact recoverable route when the close succeeds and the relevant tabs are empty;
do not perform route cleanup after a failed close.
Source: Path instructions
|
Post-rebase QA: RemoteTmuxMirrorCloseDetachTests PASSED (4.8s, TEST SUCCEEDED) on macOS with prebuilt GhosttyKit and CMUX_SKIP_ZIG_BUILD=1. Includes the RecoverableMainWindowRoute adaptation (explicit detach forgets the closed window route) — upstream window recovery landed after this branch was cut. |
|
Thanks for this, reopening closed remote tmux tabs is a great addition! One thing before it can land: it flips the |
teamleaderleo
added
enhancement
|
Thanks for this! Before we can land it, please comment |
|
Caution Pull Request opener is not an author or co-author of any commit in this PR.
This check is blocked to guard against commits being submitted under a trusted identity the submitter does not control. If this PR is a legitimate cherry-pick, release-engineering submission, or mailing-list-style patch delivery, the repository maintainer can opt out of this check by setting
I have read the CLA Document v2.2 and I hereby sign the CLA 0 out of 2 committers have signed the CLA. Warning 1 commit in this PR was authored by an email address that is not linked to any GitHub user, so we cannot tell whether the author has signed the CLA. Unlinked author:
To unblock this PR, do one of the following:
|
This branch has not been deployed
2 participants
Behavior
Scope
Focused remote-reopen change, rebased onto current upstream
main(2757d9de8). Original pre-rebase tip is preserved in Arthur-CWW/cmux asattic/pre-rebase-remote-tmux-reopen-20260728.Verification
The prepared branch included
RemoteTmuxMirrorCloseDetachTests.swift. Post-rebase focused QA is running separately; no unobserved pass claim is made here.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Restore closed remote tmux tabs by stable host/session identity so an intentional close can be reopened without killing or duplicating the remote session. Reopen prefers session id, coalesces with any live mirror, restores window and tab index, and supports async pending restores.
Written for commit 45cefd9. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests