Restore previous sessions and resume agents#2978
Merged
lawrencecchen merged 26 commits intomainfrom Apr 22, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds “Reopen Previous Session” end-to-end: CLI restore-session, UI/menu/shortcut wiring, manual snapshot caching, per-agent hook persistence with optional launchCommand, launch-context capture/reconstruction, resume-command generation, workspace/panel restore plumbing, and tests to validate relaunch resume behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as "UI / Menu / Shortcut"
participant CLI as "cmux CLI"
participant Socket as "cmux Socket"
participant App as "AppDelegate"
participant SessionStore as "SessionPersistence"
participant Index as "RestorableAgentSessionIndex"
participant Workspace as "Workspace"
participant Terminal as "TerminalPanel"
participant Agent as "Agent (Claude/Codex/OpenCode)"
User->>UI: Trigger Reopen Previous Session (⌘⇧O / menu / CLI)
UI->>App: reopenPreviousSession()
App->>SessionStore: loadReopenSessionSnapshot()
SessionStore-->>App: snapshot
App->>Index: load() (reads hook-store JSONs)
Index-->>App: restorable agent snapshots
App->>Workspace: restoreSessionSnapshot(snapshot)
Workspace->>Index: snapshot(workspaceId,panelId)
Index-->>Workspace: SessionRestorableAgentSnapshot?
Workspace->>Terminal: create panel and set working directory
alt resumeCommand present
Workspace->>Terminal: queue resumeCommand (no timeout)
Terminal->>Agent: execute resume command
Agent-->>Terminal: resumed output (scrollback)
end
App-->>User: return OK / JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Sources/ContentView.swift (1)
2043-2046:⚠️ Potential issue | 🟡 MinorUse
any Panelto clear the Swift existential warning.CI is warning that protocol existential usage will become an error in a future Swift language mode. At line 2044, change
for panel: Panel,tofor panel: any Panel,to comply with modern Swift syntax.Proposed fix
static func tmuxWorkspacePaneExactRect( - for panel: Panel, + for panel: any Panel, in contentView: NSView ) -> CGRect? {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 2043 - 2046, Update the function signature of tmuxWorkspacePaneExactRect to use the Swift 'any' existential for the Panel parameter (change the parameter from 'panel: Panel' to 'panel: any Panel') to eliminate the future Swift-language-mode existential warning; locate the function tmuxWorkspacePaneExactRect(for:in:) and adjust its first parameter type to 'any Panel' accordingly.Sources/SessionPersistence.swift (1)
331-346:⚠️ Potential issue | 🟠 MajorKeep workspace IDs in persisted snapshots.
SessionWorkspaceSnapshotis missing the optionalid, so restored workspaces lose their persisted identity instead of passing it through to the restore boundary.Proposed fix
struct SessionWorkspaceSnapshot: Codable, Sendable { + var id: UUID? var processTitle: String var customTitle: String? var customDescription: String?Based on learnings:
SessionWorkspaceSnapshot.id is declared var id: UUID? (optional) intentionally for backwards compatibility ... New snapshots always include the id.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SessionPersistence.swift` around lines 331 - 346, SessionWorkspaceSnapshot is missing the optional persisted identifier so restored workspaces lose their identity; add a var id: UUID? property to the struct (keeping it optional for backwards compatibility), ensure it is included in the Codable/Sendable struct declaration, and update any snapshot creation/encoding paths (where SessionWorkspaceSnapshot instances are constructed) to populate id for new snapshots so the restore boundary receives the persisted id.Sources/AppDelegate.swift (2)
3918-3924:⚠️ Potential issue | 🟠 MajorKeep the restored primary window active after creating extra windows.
createMainWindow(sessionWindowSnapshot:)makes each new window key/front, so multi-window restore can leave the last additional window active. Line 3974 then re-sorts key-first and re-activates that last-created window instead of the snapshot primary window. Track the first restored window and re-focus it after all additional windows are created.🐛 Proposed fix
- DispatchQueue.main.async { [weak self] in + DispatchQueue.main.async { [weak self, weak primaryWindow] in guard let self else { return } for windowSnapshot in additionalWindows { _ = self.createMainWindow(sessionWindowSnapshot: windowSnapshot) } + if let primaryWindow { + primaryWindow.makeKeyAndOrderFront(nil) + self.setActiveMainWindow(primaryWindow) + } self.completeSessionRestoreOperation() }let existingContexts = sortedMainWindowContextsForSessionSnapshot() + var primaryRestoredWindow: NSWindow? isApplyingSessionRestore = true if existingContexts.isEmpty { - for windowSnapshot in snapshotWindows { - _ = createMainWindow(sessionWindowSnapshot: windowSnapshot) + for (index, windowSnapshot) in snapshotWindows.enumerated() { + let windowId = createMainWindow(sessionWindowSnapshot: windowSnapshot) + if index == 0 { + primaryRestoredWindow = windowForMainWindowId(windowId) + } } } else { + primaryRestoredWindow = existingContexts.first.flatMap { + $0.window ?? windowForMainWindowId($0.windowId) + } for (context, windowSnapshot) in zip(existingContexts, snapshotWindows) { applySessionWindowSnapshot( windowSnapshot, to: context, window: context.window ?? windowForMainWindowId(context.windowId) @@ completeSessionRestoreOperation() - if let primaryWindow = sortedMainWindowContextsForSessionSnapshot() + if let primaryWindow = primaryRestoredWindow ?? sortedMainWindowContextsForSessionSnapshot() .compactMap({ $0.window ?? windowForMainWindowId($0.windowId) }) .first {Also applies to: 3952-3978
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 3918 - 3924, When restoring multiple windows, createMainWindow(sessionWindowSnapshot:) makes each window key so the last additional window ends up active; change the restore loop that iterates additionalWindows to capture the first restored primary window (e.g., store the result of createMainWindow(sessionWindowSnapshot:) into a variable like restoredPrimaryWindow if it is the snapshot primary), continue creating the remaining additional windows as before, then after the loop and before calling completeSessionRestoreOperation() explicitly re-focus/makeKeyAndOrderFront the captured restoredPrimaryWindow (if non-nil) so the original primary snapshot remains active; ensure this change is applied in the same restore blocks that call createMainWindow(sessionWindowSnapshot:) and completeSessionRestoreOperation().
4768-4786:⚠️ Potential issue | 🟡 MinorInclude key-window ordering in the autosave fingerprint.
buildSessionSnapshotnow serializes windows using key-first ordering, butsessionAutosaveFingerprintstill hashes UUID order and omits key-window state. A key-window-only change can be skipped as “unchanged,” leaving the persisted primary window stale until another save is forced.🔧 Proposed fix
- let contexts = mainWindowContexts.values.sorted { lhs, rhs in - lhs.windowId.uuidString < rhs.windowId.uuidString - } + let contexts = sortedMainWindowContextsForSessionSnapshot() hasher.combine(contexts.count) for context in contexts.prefix(SessionPersistencePolicy.maxWindowsPerSnapshot) { hasher.combine(context.windowId) + let window = context.window ?? windowForMainWindowId(context.windowId) + hasher.combine(window?.isKeyWindow ?? false) hasher.combine(context.tabManager.sessionAutosaveFingerprint()) hasher.combine(context.sidebarState.isVisible) @@ - if let window = context.window ?? windowForMainWindowId(context.windowId) { + if let window { Self.hashFrame(window.frame, into: &hasher) } else { hasher.combine(-1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 4768 - 4786, sessionAutosaveFingerprint is still computed from the raw UUID order and omits key-window state, so key-window-only changes won't change the fingerprint; update sessionAutosaveFingerprint to derive its ordering and data from sortedMainWindowContextsForSessionSnapshot() (the same order used by buildSessionSnapshot) and include each MainWindowContext's identity plus key-state (e.g. window?.isKeyWindow or the context's key flag) when building the hash so the fingerprint changes deterministically when the key window changes.Sources/Workspace.swift (2)
456-493:⚠️ Potential issue | 🟠 MajorPrefer the agent launch cwd when snapshotting restored agent panels.
panelDirectories[panelId]can reflect a later shell/tool cwd, so it currently overrideseffectiveRestorableAgent?.workingDirectoryand then gets persisted intoterminal.workingDirectory. For restored Claude/Codex sessions, prefer the agent snapshot cwd when present so relaunches resume from the original session directory.Proposed fix
- let directory = panelDirectories[panelId] ?? effectiveRestorableAgent?.workingDirectory + let directory = effectiveRestorableAgent?.workingDirectory ?? panelDirectories[panelId]Based on learnings: Session index cwd extraction should use the first non-empty launch cwd so tool-call cd’s do not override the launch cwd used for resumes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 456 - 493, The snapshot is currently using panelDirectories[panelId] before effectiveRestorableAgent?.workingDirectory which lets later in-session cwd changes override an agent's original launch cwd; change the selection logic so you prefer the agent launch cwd when present (e.g. use effectiveRestorableAgent?.workingDirectory if non-nil/non-empty, otherwise fall back to panelDirectories[panelId]), ensuring the value saved into terminal.workingDirectory (and related SessionTerminalPanelSnapshot) preserves the agent's original launch directory for restored Claude/Codex sessions; also ensure you treat empty strings as missing so the first non-empty launch cwd is used.
11200-11200:⚠️ Potential issue | 🔴 CriticalAdd
@MainActorisolation to theBonsplitDelegateconformance.Swift 6 requires that when an
@MainActorclass conforms to a protocol with nonisolated methods, the conformance must explicitly declare the isolation boundary. ThesplitTabBar(_:shouldCloseTab:inPane:)method at line 11641 and other protocol requirements currently lack isolation annotations, which causes a Swift 6 error.Fix
-extension Workspace: BonsplitDelegate { +extension Workspace: `@MainActor` BonsplitDelegate {This applies to all protocol requirements in the conformance (lines 11200–11724).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` at line 11200, The Workspace conformance to BonsplitDelegate needs an explicit MainActor isolation; add `@MainActor` to the extension declaration "extension Workspace: BonsplitDelegate" (or annotate each protocol requirement such as splitTabBar(_:shouldCloseTab:inPane:) and other methods in that block with `@MainActor`) so the protocol requirements are isolated to the main actor and satisfy Swift 6's isolation rules.
♻️ Duplicate comments (1)
tests/test_session_relaunch_resumes_agent_sessions.py (1)
179-197:⚠️ Potential issue | 🟠 MajorSame destructive cleanup of real user hook state as the sibling test.
codex_hook_stateandclaude_hook_statepoint at~/.cmuxterm/*-hook-sessions.json, and lines 196–197 / 278–279 unconditionallyunlinkthem. A contributor with live agent sessions loses their entire persisted hook store on every run. See the detailed recommendation on the Codex sibling test (back up + restore infinally, or redirect the hook-store directory via env var/HOMEoverride).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_session_relaunch_resumes_agent_sessions.py` around lines 179 - 197, The test unconditionally removes real user hook state files referenced by codex_hook_state and claude_hook_state; instead, ensure the test does not touch real user data by creating and using an isolated hook-store directory (e.g., set HOME or a specific env var used by the code under test to point to the tempfile.TemporaryDirectory) or by backing up any existing files and restoring them in a finally block; locate the cleanup and setup around codex_hook_state, claude_hook_state, _write_fake_agent and the TemporaryDirectory context and either (A) redirect the hook-store path into the temp dir for the duration of the test, or (B) if you must operate on the real path, move existing files to a safe backup before unlinking and restore them in finally to guarantee no user data is lost.
🧹 Nitpick comments (5)
README.md (1)
240-255: Consider syncing localized READMEs for the new restore behavior.Since this introduces user-visible behavior/shortcut docs, it would be good to mirror this section in translated README files to avoid doc drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 240 - 255, Update localized README files to include the new "Session restore" section and the user-visible shortcuts/commands (`File > Reopen Previous Session`, `⌘ ⇧ O`, `cmux restore-session`) so translations reflect the new restore behavior; locate the "Session restore" block in README.md and mirror its content (layout, working directories, scrollback, browser history, saved Claude Code/Codex resume behavior and the caveat about in-process state) into each translated README, ensuring translation teams or i18n files receive the new strings for localization.tests/test_restore_session_relaunches_codex_resume.py (2)
236-236: Magic 9.5s sleep before invokingrestore-session.Opaque fixed delay — easy to under-sleep on slow CI and over-sleep on fast machines. If this is waiting for a snapshot write after blank launch, poll for the expected
snapshot/previous_snapshotmtime or contents with_wait_for_conditioninstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_restore_session_relaunches_codex_resume.py` at line 236, Replace the opaque fixed sleep call (time.sleep(9.5)) before invoking restore-session with a polling loop using the existing test helper _wait_for_condition: check for the expected snapshot file/previous_snapshot mtime or its expected contents instead of sleeping, and only proceed to call restore-session once _wait_for_condition confirms the snapshot write; update the test around the time.sleep call to use _wait_for_condition targeting the snapshot/previous_snapshot path used in the test (or the function that reads it) so CI timing variance is handled reliably.
120-121: Reaching intoclient._send_command(private API).
_send_commandis a leading-underscore method oncmux. If that API is considered private, consider adding (or using) a publicread_screen(scrollback: bool)wrapper on the client — both new tests plus any future ones will benefit. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_restore_session_relaunches_codex_resume.py` around lines 120 - 121, The test reaches into the private API by calling cmux._send_command from _read_scrollback; add or use a public wrapper on cmux (e.g., a method named read_screen(scrollback: bool) or read_screen()) and update _read_scrollback to call that public method instead of _send_command; ensure the new public method delegates to the existing command logic so tests and future code can rely on the stable read_screen API rather than the private _send_command.Sources/RestorableAgentSession.swift (1)
76-80: Silent swallow of decode errors.
try?on bothData(contentsOf:)anddecode(...)collapses "file missing" (expected) with "file corrupt/malformed" (unexpected). On corruption the user loses resume without any signal. Consider a DEBUG-gateddlogfor decode failures so the failure mode is diagnosable from logs.♻️ Suggested sketch
- guard fileManager.fileExists(atPath: fileURL.path), - let data = try? Data(contentsOf: fileURL), - let state = try? decoder.decode(RestorableAgentHookSessionStoreFile.self, from: data) else { - continue - } + guard fileManager.fileExists(atPath: fileURL.path) else { continue } + do { + let data = try Data(contentsOf: fileURL) + let state = try decoder.decode(RestorableAgentHookSessionStoreFile.self, from: data) + // ... existing loop body, extracted ... + } catch { + `#if` DEBUG + dlog("RestorableAgentSessionIndex: failed to load \(fileURL.lastPathComponent): \(error)") + `#endif` + continue + }As per coding guidelines: "Always wrap debug event log statements in
#if DEBUG/#endifblocks. Use the free functiondlog(\"message\")…"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 76 - 80, The guard currently silences both missing-file and decode errors by using try? for Data(contentsOf:) and decoder.decode(_:), so change the logic in RestorableAgentSession where fileURL is read and decoded: explicitly read Data(contentsOf: fileURL) and call decoder.decode(RestorableAgentHookSessionStoreFile.self, from: data) inside a do/catch; on decode failures emit a debug-only log using dlog(...) wrapped in `#if` DEBUG / `#endif` that includes fileURL.path and the caught error, but continue (skip) for non-existent files as before. Ensure you reference the same symbols (fileURL, Data(contentsOf:), decoder.decode(...), RestorableAgentHookSessionStoreFile) when making the change.tests/test_session_relaunch_resumes_agent_sessions.py (1)
27-135: Extract shared test helpers to eliminate duplication.Almost all helpers here (
_bundle_id,_snapshot_path,_socket_reachable,_wait_for_socket,_wait_for_socket_closed,_kill_existing,_launch,_quit,_connect,_read_scrollback,_wait_for_condition,_write_hook_state) are byte-identical to those intests/test_restore_session_relaunches_codex_resume.py. Future drift between the two copies will cause subtle behavioral differences that are painful to debug. Consider extracting them into atests/_cmux_app_harness.py(or similar) module imported by both scripts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_session_relaunch_resumes_agent_sessions.py` around lines 27 - 135, The test helpers (functions like _bundle_id, _snapshot_path, _socket_reachable, _wait_for_socket, _wait_for_socket_closed, _kill_existing, _launch, _quit, _connect, _read_scrollback, _wait_for_condition, and _write_hook_state) are duplicated in another test file; extract them into a single shared module (e.g. tests/_cmux_app_harness.py), move the implementations there, export them with the same names, and update both test files to import these helpers instead of keeping local copies (replace local definitions with from tests._cmux_app_harness import _bundle_id, _snapshot_path, _socket_reachable, _wait_for_socket, _wait_for_socket_closed, _kill_existing, _launch, _quit, _connect, _read_scrollback, _wait_for_condition, _write_hook_state); ensure behavior and signatures remain identical and run the tests to confirm no regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 3168-3179: The current flow returns success immediately after
failing SocketClient(path: socketPath).connect(); update the restore path so
that after client.close() and after calling launchApp() and activateApp() you
wait for the app to accept connections (retry or timeout the
SocketClient.connect()), then open an RPC session and invoke the
session.restore_previous RPC and verify its response before printing success;
propagate errors if connection or RPC call fails, and only print
jsonString(["restored": true, "launched": true]) or "OK" when the restore RPC
confirmed success (use existing symbols SocketClient, client.connect(),
client.close(), launchApp(), activateApp(), session.restore_previous, and
jsonOutput to determine output format).
In `@GhosttyTabs.xcodeproj/project.pbxproj`:
- Line 864: The failing compile is due to two stale calls in
cmuxTests/TabManagerSessionSnapshotTests.swift that call
TabManager.sessionSnapshot(includeScrollback:) without the now-required
restorableAgentIndex: parameter; update both calls (the ones at the two
occurrences in that test file) to pass an appropriate restorableAgentIndex value
(e.g., 0 or a meaningful test-specific index) when invoking
TabManager.sessionSnapshot(includeScrollback:restorableAgentIndex:). Also review
and consider unifying the optionality between
TabManager.sessionSnapshot(restorableAgentIndex:) and
Workspace.sessionSnapshot(restorableAgentIndex:) so their signatures are
consistent (either make both require the parameter or both make it optional) and
adjust tests accordingly.
In `@Sources/RestorableAgentSession.swift`:
- Around line 96-99: The current merge into resolved[(workspaceId,panelId)] uses
existing.updatedAt > record.updatedAt so when updatedAt is equal the first-seen
entry wins non-deterministically; update the logic in the block around resolved,
key, snapshot, and record.updatedAt to handle ties deterministically — either
change the comparison to use >= (if you want later-seen to win) i.e. if let
existing = resolved[key], existing.updatedAt >= record.updatedAt { continue } or
implement a tie-breaker that compares snapshot.kind (or record.kind) using a
defined priority order (e.g., kindPriority(kindA) < kindPriority(kindB)) and
only keep the incoming record when it wins that tie-break; ensure you update the
assignment resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt)
accordingly.
- Around line 11-19: resumeCommand currently returns just "claude --resume <id>"
/ "codex resume <id>" and must be wrapped with a cwd guard when a
workingDirectory is present; update RestorableAgentSession.resumeCommand to
check for a workingDirectory and, if set, prepend "cd <shell-quoted cwd> && " to
the existing resume string using the same single-quote escaping helper
(Self.shellSingleQuoted), preserving existing branch outputs for .claude and
.codex; apply the same change to the other resume overload at lines 37-40 (the
alternate resume builder) so both emit "cd <cwd> && <resumeCommand>" when
workingDirectory exists.
In `@Sources/SessionPersistence.swift`:
- Around line 224-228: Add a per-panel isRemoteBacked Bool to the
SessionTerminalPanelSnapshot struct and propagate its use in restore logic:
update SessionTerminalPanelSnapshot to include isRemoteBacked (optional or
default false), ensure createPanel(from:inPane:) reads this flag when
reconstructing a terminal panel so remote-backed panels do not replay local
inputs or restore ports, and when producing snapshots for remote-backed
terminals set isRemoteBacked = true, omit restore/detected commands and ensure
listeningPorts is set to [] so the restore path can reliably skip local command
replay and port restoration.
In `@Sources/TerminalController.swift`:
- Around line 130-133: The focus-intent allow-list currently includes
"session.restore_previous" in the static Set focusIntentV2Methods; remove
"session.restore_previous" from that Set so restoring sessions does not count as
an explicit focus command, and ensure any handling in TerminalController (or
related restore/session code paths) restores model/data without invoking
focus/selection mutation; look for references to focusIntentV2Methods and update
tests or callers that relied on that entry to use an explicit focus command or
an added activation parameter if focus is actually desired.
- Around line 6950-6952: The error return in the restore path (the guard
restored else block in TerminalController.swift) must use a localized
user-facing string; replace the hard-coded message "No previous session snapshot
available" with String(localized: "terminal.restore.no_snapshot", defaultValue:
"No previous session snapshot available") and keep the same .err(...) shape, and
add the corresponding key/value entry to Resources/Localizable.xcstrings
(terminal.restore.no_snapshot = "No previous session snapshot available";).
- Around line 6945-6949: Replace the raw DispatchQueue.main.sync usage in
v2SessionRestorePrevious() with the v2MainSync helper: move the
AppDelegate.shared?.reopenPreviousSession() call into a v2MainSync block (like
other v2 methods such as v2WorkspaceCreate and v2WorkspaceList), assign its
Boolean result to the local restored variable inside that block, and return
restored as before to avoid potential deadlocks when called from background
socket threads.
In `@Sources/Workspace.swift`:
- Around line 686-693: The resumeCommand is sent raw to the new terminal and can
be misdirected if shell rc files change directory; update the call site in the
restore path to guard the command with a cwd check before calling
sendInputWhenReady: obtain a cwd-guarded command from the
SessionRestorableAgentSnapshot (or add a helper mirroring
SessionEntry.resumeCommandWithCwd that returns "cd <shell-quoted cwd> &&
<resumeCommand>") and pass that guarded string (with trailing "\n") to
sendInputWhenReady for terminalPanel after applySessionPanelMetadata; keep
restoredAgentSnapshotsByPanelId and resume-command extraction logic but replace
direct use of snapshot.terminal?.agent?.resumeCommand with the new
resume-with-cwd helper.
In `@tests/test_restore_session_relaunches_codex_resume.py`:
- Around line 180-192: The test directly constructs hook_state = Path.home() /
".cmuxterm" / "codex-hook-sessions.json" and unconditionally calls
hook_state.unlink(...), which deletes real user state; change the test to avoid
touching the real HOME by overriding the environment for the app under test
(e.g., set HOME to the TemporaryDirectory or set a dedicated env var like
CMUX_HOOK_STATE_DIR used by the app) so that Path.home() (or the app's
hook-state lookup) resolves into the tempdir; alternatively implement a
backup/restore around the existing file (copy before running and restore in
finally) if environment override isn't possible — update references to
hook_state and any unlink calls to operate on the tempdir-controlled path
instead.
---
Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 3918-3924: When restoring multiple windows,
createMainWindow(sessionWindowSnapshot:) makes each window key so the last
additional window ends up active; change the restore loop that iterates
additionalWindows to capture the first restored primary window (e.g., store the
result of createMainWindow(sessionWindowSnapshot:) into a variable like
restoredPrimaryWindow if it is the snapshot primary), continue creating the
remaining additional windows as before, then after the loop and before calling
completeSessionRestoreOperation() explicitly re-focus/makeKeyAndOrderFront the
captured restoredPrimaryWindow (if non-nil) so the original primary snapshot
remains active; ensure this change is applied in the same restore blocks that
call createMainWindow(sessionWindowSnapshot:) and
completeSessionRestoreOperation().
- Around line 4768-4786: sessionAutosaveFingerprint is still computed from the
raw UUID order and omits key-window state, so key-window-only changes won't
change the fingerprint; update sessionAutosaveFingerprint to derive its ordering
and data from sortedMainWindowContextsForSessionSnapshot() (the same order used
by buildSessionSnapshot) and include each MainWindowContext's identity plus
key-state (e.g. window?.isKeyWindow or the context's key flag) when building the
hash so the fingerprint changes deterministically when the key window changes.
In `@Sources/ContentView.swift`:
- Around line 2043-2046: Update the function signature of
tmuxWorkspacePaneExactRect to use the Swift 'any' existential for the Panel
parameter (change the parameter from 'panel: Panel' to 'panel: any Panel') to
eliminate the future Swift-language-mode existential warning; locate the
function tmuxWorkspacePaneExactRect(for:in:) and adjust its first parameter type
to 'any Panel' accordingly.
In `@Sources/SessionPersistence.swift`:
- Around line 331-346: SessionWorkspaceSnapshot is missing the optional
persisted identifier so restored workspaces lose their identity; add a var id:
UUID? property to the struct (keeping it optional for backwards compatibility),
ensure it is included in the Codable/Sendable struct declaration, and update any
snapshot creation/encoding paths (where SessionWorkspaceSnapshot instances are
constructed) to populate id for new snapshots so the restore boundary receives
the persisted id.
In `@Sources/Workspace.swift`:
- Around line 456-493: The snapshot is currently using panelDirectories[panelId]
before effectiveRestorableAgent?.workingDirectory which lets later in-session
cwd changes override an agent's original launch cwd; change the selection logic
so you prefer the agent launch cwd when present (e.g. use
effectiveRestorableAgent?.workingDirectory if non-nil/non-empty, otherwise fall
back to panelDirectories[panelId]), ensuring the value saved into
terminal.workingDirectory (and related SessionTerminalPanelSnapshot) preserves
the agent's original launch directory for restored Claude/Codex sessions; also
ensure you treat empty strings as missing so the first non-empty launch cwd is
used.
- Line 11200: The Workspace conformance to BonsplitDelegate needs an explicit
MainActor isolation; add `@MainActor` to the extension declaration "extension
Workspace: BonsplitDelegate" (or annotate each protocol requirement such as
splitTabBar(_:shouldCloseTab:inPane:) and other methods in that block with
`@MainActor`) so the protocol requirements are isolated to the main actor and
satisfy Swift 6's isolation rules.
---
Duplicate comments:
In `@tests/test_session_relaunch_resumes_agent_sessions.py`:
- Around line 179-197: The test unconditionally removes real user hook state
files referenced by codex_hook_state and claude_hook_state; instead, ensure the
test does not touch real user data by creating and using an isolated hook-store
directory (e.g., set HOME or a specific env var used by the code under test to
point to the tempfile.TemporaryDirectory) or by backing up any existing files
and restoring them in a finally block; locate the cleanup and setup around
codex_hook_state, claude_hook_state, _write_fake_agent and the
TemporaryDirectory context and either (A) redirect the hook-store path into the
temp dir for the duration of the test, or (B) if you must operate on the real
path, move existing files to a safe backup before unlinking and restore them in
finally to guarantee no user data is lost.
---
Nitpick comments:
In `@README.md`:
- Around line 240-255: Update localized README files to include the new "Session
restore" section and the user-visible shortcuts/commands (`File > Reopen
Previous Session`, `⌘ ⇧ O`, `cmux restore-session`) so translations reflect the
new restore behavior; locate the "Session restore" block in README.md and mirror
its content (layout, working directories, scrollback, browser history, saved
Claude Code/Codex resume behavior and the caveat about in-process state) into
each translated README, ensuring translation teams or i18n files receive the new
strings for localization.
In `@Sources/RestorableAgentSession.swift`:
- Around line 76-80: The guard currently silences both missing-file and decode
errors by using try? for Data(contentsOf:) and decoder.decode(_:), so change the
logic in RestorableAgentSession where fileURL is read and decoded: explicitly
read Data(contentsOf: fileURL) and call
decoder.decode(RestorableAgentHookSessionStoreFile.self, from: data) inside a
do/catch; on decode failures emit a debug-only log using dlog(...) wrapped in
`#if` DEBUG / `#endif` that includes fileURL.path and the caught error, but continue
(skip) for non-existent files as before. Ensure you reference the same symbols
(fileURL, Data(contentsOf:), decoder.decode(...),
RestorableAgentHookSessionStoreFile) when making the change.
In `@tests/test_restore_session_relaunches_codex_resume.py`:
- Line 236: Replace the opaque fixed sleep call (time.sleep(9.5)) before
invoking restore-session with a polling loop using the existing test helper
_wait_for_condition: check for the expected snapshot file/previous_snapshot
mtime or its expected contents instead of sleeping, and only proceed to call
restore-session once _wait_for_condition confirms the snapshot write; update the
test around the time.sleep call to use _wait_for_condition targeting the
snapshot/previous_snapshot path used in the test (or the function that reads it)
so CI timing variance is handled reliably.
- Around line 120-121: The test reaches into the private API by calling
cmux._send_command from _read_scrollback; add or use a public wrapper on cmux
(e.g., a method named read_screen(scrollback: bool) or read_screen()) and update
_read_scrollback to call that public method instead of _send_command; ensure the
new public method delegates to the existing command logic so tests and future
code can rely on the stable read_screen API rather than the private
_send_command.
In `@tests/test_session_relaunch_resumes_agent_sessions.py`:
- Around line 27-135: The test helpers (functions like _bundle_id,
_snapshot_path, _socket_reachable, _wait_for_socket, _wait_for_socket_closed,
_kill_existing, _launch, _quit, _connect, _read_scrollback, _wait_for_condition,
and _write_hook_state) are duplicated in another test file; extract them into a
single shared module (e.g. tests/_cmux_app_harness.py), move the implementations
there, export them with the same names, and update both test files to import
these helpers instead of keeping local copies (replace local definitions with
from tests._cmux_app_harness import _bundle_id, _snapshot_path,
_socket_reachable, _wait_for_socket, _wait_for_socket_closed, _kill_existing,
_launch, _quit, _connect, _read_scrollback, _wait_for_condition,
_write_hook_state); ensure behavior and signatures remain identical and run the
tests to confirm no regressions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a56a50c0-bb5a-4eab-9a9f-f5b4c18fc657
📒 Files selected for processing (17)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojREADME.mdResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/RestorableAgentSession.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swiftSources/cmuxApp.swifttests/test_restore_session_relaunches_codex_resume.pytests/test_session_relaunch_resumes_agent_sessions.pyweb/data/cmux-settings.schema.jsonweb/data/cmux-shortcuts.ts
Comment on lines
+96
to
+99
| if let existing = resolved[key], existing.updatedAt > record.updatedAt { | ||
| continue | ||
| } | ||
| resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt) |
There was a problem hiding this comment.
Duplicate updatedAt ties keep arbitrary record.
When two records for the same (workspaceId, panelId) key share updatedAt (e.g., both written within the same second, or same store key appearing twice across kinds — note this map is shared across kinds), the first-seen entry wins non-deterministically depending on dictionary iteration order. Consider >= to prefer the later-seen, or (more robustly) break ties by kind priority. Minor — unlikely to hit in practice but worth hardening.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/RestorableAgentSession.swift` around lines 96 - 99, The current merge
into resolved[(workspaceId,panelId)] uses existing.updatedAt > record.updatedAt
so when updatedAt is equal the first-seen entry wins non-deterministically;
update the logic in the block around resolved, key, snapshot, and
record.updatedAt to handle ties deterministically — either change the comparison
to use >= (if you want later-seen to win) i.e. if let existing = resolved[key],
existing.updatedAt >= record.updatedAt { continue } or implement a tie-breaker
that compares snapshot.kind (or record.kind) using a defined priority order
(e.g., kindPriority(kindA) < kindPriority(kindB)) and only keep the incoming
record when it wins that tie-break; ensure you update the assignment
resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt) accordingly.
Comment on lines
224
to
228
| struct SessionTerminalPanelSnapshot: Codable, Sendable { | ||
| var workingDirectory: String? | ||
| var scrollback: String? | ||
| var agent: SessionRestorableAgentSnapshot? | ||
| } |
There was a problem hiding this comment.
Preserve the per-panel remote-backed flag in terminal snapshots.
agent is fine as an optional additive field, but this snapshot still needs isRemoteBacked; otherwise restore code cannot reliably avoid replaying local inputs/commands or restoring ports for remote-backed terminals.
Proposed fix
struct SessionTerminalPanelSnapshot: Codable, Sendable {
var workingDirectory: String?
var scrollback: String?
+ var isRemoteBacked: Bool?
var agent: SessionRestorableAgentSnapshot?
}Based on learnings: SessionTerminalPanelSnapshot now includes isRemoteBacked. createPanel(from:inPane:) uses this per-panel flag ... Remote-backed terminals also omit restore/detected commands and have listeningPorts [] in the snapshot.
📝 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.
Suggested change
| struct SessionTerminalPanelSnapshot: Codable, Sendable { | |
| var workingDirectory: String? | |
| var scrollback: String? | |
| var agent: SessionRestorableAgentSnapshot? | |
| } | |
| struct SessionTerminalPanelSnapshot: Codable, Sendable { | |
| var workingDirectory: String? | |
| var scrollback: String? | |
| var isRemoteBacked: Bool? | |
| var agent: SessionRestorableAgentSnapshot? | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/SessionPersistence.swift` around lines 224 - 228, Add a per-panel
isRemoteBacked Bool to the SessionTerminalPanelSnapshot struct and propagate its
use in restore logic: update SessionTerminalPanelSnapshot to include
isRemoteBacked (optional or default false), ensure createPanel(from:inPane:)
reads this flag when reconstructing a terminal panel so remote-backed panels do
not replay local inputs or restore ports, and when producing snapshots for
remote-backed terminals set isRemoteBacked = true, omit restore/detected
commands and ensure listeningPorts is set to [] so the restore path can reliably
skip local command replay and port restoration.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR adds full session restore: layout, scrollback, working directories, and resume tokens for Claude Code and Codex panels are all embedded in the session snapshot on quit and replayed on relaunch (
Confidence Score: 4/5Safe to merge after fixing the socket focus policy violation; all other changes are well-structured. One P1 issue: the session.restore_previous socket handler calls reopenPreviousSession() which unconditionally steals macOS app focus, violating the documented socket focus policy. The rest of the implementation — agent snapshot embedding, -previous snapshot cache, keyboard shortcut wiring, localization, and E2E tests — is clean and correct. Sources/TerminalController.swift (v2SessionRestorePrevious focus stealing) and Sources/AppDelegate.swift (reopenPreviousSession activation guard) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant Workspace
participant HookFiles as Hook Files
participant SnapshotFile as Snapshot File
participant Terminal
Note over User,Terminal: On quit (session save)
AppDelegate->>HookFiles: RestorableAgentSessionIndex.load()
HookFiles-->>AppDelegate: index workspaceId+panelId to agentSnapshot
AppDelegate->>Workspace: sessionSnapshot(restorableAgentIndex)
Workspace->>SnapshotFile: embed agent.kind+sessionId per panel
SnapshotFile-->>AppDelegate: saved (agent tokens embedded)
Note over User,Terminal: On relaunch (startup restore)
AppDelegate->>SnapshotFile: syncManualRestoreSnapshotCache() copy to -previous
AppDelegate->>SnapshotFile: load() startupSessionSnapshot
AppDelegate->>Workspace: restoreSessionSnapshot(snapshot)
Workspace->>Workspace: createPanel stores agent in restoredAgentSnapshotsByPanelId
Workspace->>Terminal: sendInputWhenReady claude --resume TOKEN
Terminal-->>User: resumed session
Note over User,Terminal: Manual reopen (menu / cmd+shift+O / cmux restore-session)
User->>AppDelegate: reopenPreviousSession()
AppDelegate->>SnapshotFile: loadReopenSessionSnapshot() -previous or default
AppDelegate->>Workspace: restoreSessionSnapshot(snapshot)
Workspace->>Terminal: sendInputWhenReady(resumeCommand)
AppDelegate->>AppDelegate: activate app windows (skipped for socket callers)
Reviews (1): Last reviewed commit: "Add relaunch regression coverage for ses..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+692
to
+694
| if let resumeCommand = snapshot.terminal?.agent?.resumeCommand { | ||
| sendInputWhenReady(resumeCommand + "\n", to: terminalPanel) | ||
| } |
Contributor
There was a problem hiding this comment.
sendInputWhenReady drops the command with only an NSLog if the surface isn't ready within 3 seconds. This is fine for single panels, but a full session restore that creates many terminals simultaneously could exhaust that budget on a slow or loaded machine — and the user would see a normal shell instead of their resumed agent with no indication of what happened.
Consider logging a user-visible warning (e.g., via dlog in debug or a log entry in the workspace sidebar) when the timeout fires for a restorable-agent panel, so the failure is discoverable.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:3957">
P2: Reopening a smaller saved session does not remove extra current windows, so the restored session is incorrect and the stale windows get re-saved.</violation>
</file>
<file name="web/data/cmux-shortcuts.ts">
<violation number="1" location="web/data/cmux-shortcuts.ts:51">
P3: Move this shortcut out of the workspace category; it represents an app/window-level restore action, so the web shortcuts data now disagrees with the rest of the docs.</violation>
</file>
<file name="tests/test_restore_session_relaunches_codex_resume.py">
<violation number="1" location="tests/test_restore_session_relaunches_codex_resume.py:192">
P2: The test unconditionally deletes `~/.cmuxterm/codex-hook-sessions.json` (the real user-level hook store, not a test fixture). Running this test locally destroys any persisted Codex hook sessions. Back up the file before test and restore it in `finally`, or redirect the hook-state directory via an env var so the test never touches the real store.</violation>
<violation number="2" location="tests/test_restore_session_relaunches_codex_resume.py:220">
P2: Clear the Codex hook state before the blank relaunch so the regression actually exercises the saved session snapshot.</violation>
</file>
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:3171">
P2: The not-running fallback ignores the requested socket/app instance and always launches the default `cmux` app.</violation>
<violation number="2" location="CLI/cmux.swift:3174">
P2: This branch reports a successful restore without verifying that any previous session was actually restored.</violation>
</file>
<file name="Sources/TabManager.swift">
<violation number="1" location="Sources/TabManager.swift:6788">
P2: Include restorable-agent resume metadata in the autosave fingerprint. Right now autosave can treat a changed snapshot as unchanged and delay saving newly resumable Claude/Codex sessions.</violation>
</file>
<file name="Sources/Workspace.swift">
<violation number="1" location="Sources/Workspace.swift:449">
P1: Falling back to `restoredAgentSnapshotsByPanelId` keeps stale agent metadata alive after the agent session has ended, so later restores can wrongly relaunch an old Claude/Codex session in a reused terminal.</violation>
</file>
<file name="Sources/SessionPersistence.swift">
<violation number="1" location="Sources/SessionPersistence.swift:409">
P1: `loadReopenSessionSnapshot` falls back to `load()` (the active/current session snapshot) when the `-previous` snapshot file is missing. If the `-previous` file was never written (e.g., disk-full, first launch, or permission error), calling `reopenPreviousSession` will restore the *current* session's state on top of existing windows, duplicating workspaces and panels instead of restoring a prior session.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
3 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lawrencecchen
force-pushed
the
issue-2923-reopen-sessions
branch
from
f4f2728 to
8fc8cce
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
250-275:⚠️ Potential issue | 🟠 MajorAvoid persisting stale restored agents when an index is present but empty.
restorableAgentIndex?.snapshot(...)collapses two cases intonil: no index was provided, or the latest index explicitly has no agent for this panel. BecausesessionPanelSnapshotthen falls back torestoredAgentSnapshotsByPanelId, a previously restored agent can be re-saved after it is no longer active, causing a later restore to resume an old session unexpectedly.Suggested cache invalidation shape
let panelSnapshots = allPanelIds .prefix(SessionPersistencePolicy.maxPanelsPerWorkspace) .compactMap { panelId in + let indexedRestorableAgent = restorableAgentIndex?.snapshot(workspaceId: id, panelId: panelId) sessionPanelSnapshot( panelId: panelId, includeScrollback: includeScrollback, - restorableAgent: restorableAgentIndex?.snapshot(workspaceId: id, panelId: panelId) + restorableAgent: indexedRestorableAgent, + restorableAgentIndexProvided: restorableAgentIndex != nil ) }private func sessionPanelSnapshot( panelId: UUID, includeScrollback: Bool, - restorableAgent: SessionRestorableAgentSnapshot? + restorableAgent: SessionRestorableAgentSnapshot?, + restorableAgentIndexProvided: Bool ) -> SessionPanelSnapshot? { guard let panel = panels[panelId] else { return nil } - let effectiveRestorableAgent = restorableAgent ?? restoredAgentSnapshotsByPanelId[panelId] + let effectiveRestorableAgent: SessionRestorableAgentSnapshot? if let restorableAgent { restoredAgentSnapshotsByPanelId[panelId] = restorableAgent + effectiveRestorableAgent = restorableAgent + } else if restorableAgentIndexProvided { + restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) + effectiveRestorableAgent = nil + } else { + effectiveRestorableAgent = restoredAgentSnapshotsByPanelId[panelId] }Also applies to: 442-452
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 250 - 275, The snapshot code must distinguish "no index provided" from "index provided but contains no agent" so we don't re-persist stale restored agents; change the call site in sessionSnapshot where you compute restorableAgent: replace the inline restorableAgentIndex?.snapshot(...) with a two-step calculation: 1) compute let indexProvided = (restorableAgentIndex != nil), 2) compute let restorableAgentSnapshot = restorableAgentIndex?.snapshot(workspaceId: id, panelId: panelId). Then change the sessionPanelSnapshot signature (and its other call sites, including the 442-452 occurrence) to accept both restorableAgentSnapshot: RestorableAgentSession? and indexProvided: Bool (or an equivalent sentinel flag), and inside sessionPanelSnapshot use indexProvided==true && restorableAgentSnapshot==nil to suppress falling back to restoredAgentSnapshotsByPanelId; only fall back when indexProvided==false. This ensures an explicit empty index prevents re-saving previously restored agents.
♻️ Duplicate comments (2)
Sources/Workspace.swift (1)
686-693:⚠️ Potential issue | 🟠 MajorGuard restored resume commands with the saved cwd before injection.
resumeCommandis still sent raw to a fresh terminal; shell rc files can change directories before the input runs. Use a cwd-guarded helper, e.g.cd <shell-quoted cwd> && <resumeCommand>, before callingsendInputWhenReady.Suggested call-site shape
- if let resumeCommand = snapshot.terminal?.agent?.resumeCommand { - sendInputWhenReady(resumeCommand + "\n", to: terminalPanel) + if let restorableAgent = snapshot.terminal?.agent { + sendInputWhenReady(restorableAgent.resumeCommandWithCwd + "\n", to: terminalPanel) }Based on learnings: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry, producing
cd <shell-quoted cwd> && <resumeCommand>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 686 - 693, The resumeCommand is being sent raw to the terminal; instead, if snapshot.terminal?.agent?.resumeCommand exists, wrap it with the SessionEntry cwd-guard helper to produce a guarded invocation like `cd <shell-quoted cwd> && <resumeCommand>` using the saved cwd from snapshot.terminal (ensure proper shell-quoting of the cwd), then call sendInputWhenReady(guardedCommand + "\n", to: terminalPanel); update the block that references restoredAgentSnapshotsByPanelId, snapshot.terminal?.agent?.resumeCommand and sendInputWhenReady to use the SessionEntry-provided cwd-guard helper.CLI/cmux.swift (1)
3184-3195:⚠️ Potential issue | 🟠 MajorConfirm restore before printing success.
On socket connect failure, this launches/activates cmux and returns
OKwithout waiting for the app socket or callingsession.restore_previous, so a failed/delayed startup can be reported as a successful restore.Suggested fix
- let client = SocketClient(path: socketPath) - if (try? client.connect()) == nil { - client.close() - try launchApp() - try activateApp() - if jsonOutput { - print(jsonString(["restored": true, "launched": true])) - } else { - print("OK") - } - return - } - - defer { client.close() } + let initialClient = SocketClient(path: socketPath) + let client: SocketClient + let launched: Bool + if (try? initialClient.connect()) == nil { + initialClient.close() + try launchApp() + client = try SocketClient.waitForConnectableSocket(path: socketPath, timeout: 10) + launched = true + } else { + client = initialClient + launched = false + } + + defer { client.close() } try authenticateClientIfNeeded( client, explicitPassword: explicitPassword, socketPath: socketPath ) let response = try client.sendV2(method: "session.restore_previous") + try activateApp() if jsonOutput { - print(jsonString(response)) + var payload = response + payload["launched"] = launched + print(jsonString(payload)) } else { print("OK") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3184 - 3195, On socket connect failure in the block that creates SocketClient and calls launchApp()/activateApp(), change the flow to wait for the app to be ready and call session.restore_previous (or otherwise verify the socket becomes available) before printing success; specifically, after try launchApp() and try activateApp() use the existing SocketClient.connect (or a new retry/wait loop) to confirm the socket is open and then call session.restore_previous and only then print jsonString(["restored": true, "launched": true]) or "OK"; ensure client.close() still happens on errors and propagate failure (or print a failure result) if the socket/restore does not succeed within the timeout.
🧹 Nitpick comments (1)
Sources/RestorableAgentSession.swift (1)
584-586: Redundant explicit initializer (SwiftLintunneeded_synthesized_initializer).Since
PanelKeyis private, the synthesized memberwise initializer would already be private by Swift's access-inference rules, so this explicit init can be removed.♻️ Proposed refactor
- private init(snapshotsByPanel: [PanelKey: SessionRestorableAgentSnapshot]) { - self.snapshotsByPanel = snapshotsByPanel - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 584 - 586, The private explicit initializer private init(snapshotsByPanel: [PanelKey: SessionRestorableAgentSnapshot]) is redundant because Swift will synthesize an equivalent private memberwise initializer (PanelKey is private), so remove this init declaration and rely on the synthesized memberwise initializer; ensure no other callers rely on this explicit initializer name (search for private init(snapshotsByPanel:) or direct initialization of RestorableAgentSession with snapshotsByPanel) and run tests to confirm no access regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 13683-13703: Before constructing AgentHookLaunchCommandRecord,
validate and sanitize the decoded/process arguments (the local variables
arguments, executablePath, workingDirectory, and environment) using the
allowlist check SessionRestoreCommandSettings.isCommandAllowed(...) or an
equivalent isAgentLaunchCommandSafeToPersist(...) helper; if the check fails,
drop or redact unsafe argv entries (e.g., replace sensitive elements with
placeholders or persist only resume-safe flags/session IDs) and ensure you never
persist raw free-form argv or sensitive environment entries; then pass the
sanitized executablePath/arguments/workingDirectory/environment into
AgentHookLaunchCommandRecord so only allowlisted/resume-safe data is stored.
In `@Sources/AppDelegate.swift`:
- Around line 3949-3955: When existingContexts is empty, clear the pending
startup restore state before calling createMainWindow(sessionWindowSnapshot:) so
attemptStartupSessionRestoreIfNeeded cannot apply startupSessionSnapshot or
fallback geometry during synchronous registration; specifically, before the loop
that calls createMainWindow(sessionWindowSnapshot:) reset or nil out the
startupSessionSnapshot and any fallback geometry (or set the flag that causes
attemptStartupSessionRestoreIfNeeded to no-op) so that
attemptStartupSessionRestoreIfNeeded will not apply automatic restores while
manual restore windows are being created.
- Around line 3974-3981: Add an activateApplication: Bool = true parameter to
the restore/apply entry (the method that currently calls
sortedMainWindowContextsForSessionSnapshot() and then
primaryWindow.makeKeyAndOrderFront(nil)/setActiveMainWindow(...)/NSRunningApplication.current.activate(...)),
and only perform makeKeyAndOrderFront + setActiveMainWindow +
NSRunningApplication.current.activate when activateApplication is true; during
the apply loop capture the first restored window (do not re-sort windows after
restore) and use that captured window as the primaryWindow instead of
recomputing via
sortedMainWindowContextsForSessionSnapshot()/windowForMainWindowId, and update
the socket/CLI caller TerminalController.v2SessionRestorePrevious() to call the
new API with activateApplication: false while UI callers keep the default.
In `@Sources/RestorableAgentSession.swift`:
- Around line 197-213: The code currently appends the literal "env" to
commandParts as soon as launchCommand?.environment is non-empty, causing an
"env" prefix even when all keys are filtered out by isSafeEnvironmentKey; change
the logic in the method that builds commandParts (the block that reads
launchCommand?.environment and uses isSafeEnvironmentKey) so you collect safe
key=value pairs first and only append "env" plus those pairs to commandParts if
at least one safe pair exists (leave argv, shellSingleQuoted mapping,
cwd/normalized handling and the final shellCommand assembly unchanged).
---
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 250-275: The snapshot code must distinguish "no index provided"
from "index provided but contains no agent" so we don't re-persist stale
restored agents; change the call site in sessionSnapshot where you compute
restorableAgent: replace the inline restorableAgentIndex?.snapshot(...) with a
two-step calculation: 1) compute let indexProvided = (restorableAgentIndex !=
nil), 2) compute let restorableAgentSnapshot =
restorableAgentIndex?.snapshot(workspaceId: id, panelId: panelId). Then change
the sessionPanelSnapshot signature (and its other call sites, including the
442-452 occurrence) to accept both restorableAgentSnapshot:
RestorableAgentSession? and indexProvided: Bool (or an equivalent sentinel
flag), and inside sessionPanelSnapshot use indexProvided==true &&
restorableAgentSnapshot==nil to suppress falling back to
restoredAgentSnapshotsByPanelId; only fall back when indexProvided==false. This
ensures an explicit empty index prevents re-saving previously restored agents.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 3184-3195: On socket connect failure in the block that creates
SocketClient and calls launchApp()/activateApp(), change the flow to wait for
the app to be ready and call session.restore_previous (or otherwise verify the
socket becomes available) before printing success; specifically, after try
launchApp() and try activateApp() use the existing SocketClient.connect (or a
new retry/wait loop) to confirm the socket is open and then call
session.restore_previous and only then print jsonString(["restored": true,
"launched": true]) or "OK"; ensure client.close() still happens on errors and
propagate failure (or print a failure result) if the socket/restore does not
succeed within the timeout.
In `@Sources/Workspace.swift`:
- Around line 686-693: The resumeCommand is being sent raw to the terminal;
instead, if snapshot.terminal?.agent?.resumeCommand exists, wrap it with the
SessionEntry cwd-guard helper to produce a guarded invocation like `cd
<shell-quoted cwd> && <resumeCommand>` using the saved cwd from
snapshot.terminal (ensure proper shell-quoting of the cwd), then call
sendInputWhenReady(guardedCommand + "\n", to: terminalPanel); update the block
that references restoredAgentSnapshotsByPanelId,
snapshot.terminal?.agent?.resumeCommand and sendInputWhenReady to use the
SessionEntry-provided cwd-guard helper.
---
Nitpick comments:
In `@Sources/RestorableAgentSession.swift`:
- Around line 584-586: The private explicit initializer private
init(snapshotsByPanel: [PanelKey: SessionRestorableAgentSnapshot]) is redundant
because Swift will synthesize an equivalent private memberwise initializer
(PanelKey is private), so remove this init declaration and rely on the
synthesized memberwise initializer; ensure no other callers rely on this
explicit initializer name (search for private init(snapshotsByPanel:) or direct
initialization of RestorableAgentSession with snapshotsByPanel) and run tests to
confirm no access regressions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfe2907e-0ceb-42b6-afd1-4ac321282ef4
📒 Files selected for processing (8)
CLI/cmux.swiftResources/bin/claudeSources/AppDelegate.swiftSources/RestorableAgentSession.swiftSources/TabManager.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceTests.swifttests/test_claude_wrapper_hooks.py
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
CLI/cmux.swift (2)
13683-13703:⚠️ Potential issue | 🟠 MajorGate argv before persisting launch commands.
This still persists decoded/process
arguments,executablePath,workingDirectory, and selected environment values before a restore-safe allowlist/redaction pass. Agent argv can include prompts, tokens, or private paths that then land in the session store/snapshot.Suggested direction
let arguments = envArguments ?? processArguments guard let arguments, !arguments.isEmpty else { return nil } + guard isAgentLaunchCommandSafeToPersist( + arguments: arguments, + launcher: normalizedHookValue(env["CMUX_AGENT_LAUNCH_KIND"]) ?? fallbackKind + ) else { + return nil + } let executablePath = normalizedHookValue(env["CMUX_AGENT_LAUNCH_EXECUTABLE"]) ?? arguments.first let workingDirectory = normalizedHookValue(env["CMUX_AGENT_LAUNCH_CWD"]) ?? normalizedHookValue(cwd) ?? normalizedHookValue(env["PWD"])Based on learnings, detected foreground command lines should only be recorded if they pass
SessionRestoreCommandSettings.isCommandAllowed(...), so unallowed/sensitive commands are never written to session JSON.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 13683 - 13703, The code currently decodes and builds arguments/executablePath/workingDirectory/environment then immediately constructs AgentHookLaunchCommandRecord; instead, before creating/persisting that record run the allowlist check via SessionRestoreCommandSettings.isCommandAllowed(...) using the resolved executablePath and arguments (e.g., combine executablePath and arguments from decodeNULSeparatedBase64(...) / processArguments(for:) and pass to isCommandAllowed), and if the command is not allowed either skip creating the record or redact sensitive fields (arguments, workingDirectory, environment) to nil/placeholder; ensure the same gating is applied to environment returned by selectedAgentLaunchEnvironment(...) and any normalizedHookValue(...) results so no unallowed argv/paths/tokens are written to session JSON.
3184-3194:⚠️ Potential issue | 🟠 MajorConfirm restore before reporting success.
This still reports
OKafter launching the app without waiting for the socket or invokingsession.restore_previous, so launch/startup failure or a missing snapshot can be reported as a successful restore.Suggested fix
- let client = SocketClient(path: socketPath) - if (try? client.connect()) == nil { - client.close() - try launchApp() - try activateApp() - if jsonOutput { - print(jsonString(["restored": true, "launched": true])) - } else { - print("OK") - } - return - } - - defer { client.close() } + let initialClient = SocketClient(path: socketPath) + let launched: Bool + let client: SocketClient + if (try? initialClient.connect()) == nil { + initialClient.close() + try launchApp() + launched = true + client = try SocketClient.waitForConnectableSocket(path: socketPath, timeout: 10) + } else { + launched = false + client = initialClient + } + + defer { client.close() } try authenticateClientIfNeeded( client, explicitPassword: explicitPassword, socketPath: socketPath ) let response = try client.sendV2(method: "session.restore_previous") + try activateApp() if jsonOutput { - print(jsonString(response)) + var payload = response + payload["launched"] = launched + print(jsonString(payload)) } else { print("OK") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3184 - 3194, The current flow prints success after launching the app without confirming a restored session or a working socket; modify the branch that runs when (try? client.connect()) == nil to first wait/retry for the socket to become available (e.g., attempt client.connect() with a short retry loop and timeout), then call session.restore_previous() and check its return/result, only proceeding to print jsonString(["restored": true, "launched": true]) or "OK" when both the socket connect succeeds and session.restore_previous() indicates success; if either step fails, return an appropriate failure path (close the SocketClient via client.close() and print a failed/restored:false result). Ensure you update the code paths around SocketClient, client.connect(), client.close(), launchApp(), activateApp(), and session.restore_previous() accordingly.Sources/AppDelegate.swift (2)
3939-3981:⚠️ Potential issue | 🟠 MajorKeep socket/CLI restore non-activating and avoid re-sorting the restored primary window.
reopenPreviousSession()unconditionally raises/activates the app at Lines 3977-3981. Since this restore is also exposed throughsession.restore_previous/cmux restore-session, socket/CLI callers can steal macOS focus. Also, re-sorting contexts after restore can pick a different key/new window than the first restored snapshot window.Add an
activateApplication: Bool = trueparameter, capture the first restored window during the apply/create loop, and have the socket/CLI caller passfalse. As per coding guidelines, “Socket/CLI commands must not steal macOS app focus. Only explicit focus-intent commands (window.focus,workspace.select/next/previous/last,surface.focus,pane.focus/last, browser focus commands) may mutate in-app focus/selection”.Proposed direction
- func reopenPreviousSession() -> Bool { + func reopenPreviousSession(activateApplication: Bool = true) -> Bool { guard let snapshot = SessionPersistenceStore.loadReopenSessionSnapshot() else { return false } @@ let existingContexts = sortedMainWindowContextsForSessionSnapshot() + var primaryRestoredWindow: NSWindow? isApplyingSessionRestore = true if existingContexts.isEmpty { for windowSnapshot in snapshotWindows { - _ = createMainWindow(sessionWindowSnapshot: windowSnapshot) + let windowId = createMainWindow(sessionWindowSnapshot: windowSnapshot) + if primaryRestoredWindow == nil { + primaryRestoredWindow = windowForMainWindowId(windowId) + } } } else { for (context, windowSnapshot) in zip(existingContexts, snapshotWindows) { + let targetWindow = context.window ?? windowForMainWindowId(context.windowId) + if primaryRestoredWindow == nil { + primaryRestoredWindow = targetWindow + } applySessionWindowSnapshot( windowSnapshot, to: context, - window: context.window ?? windowForMainWindowId(context.windowId) + window: targetWindow ) } @@ completeSessionRestoreOperation() - if let primaryWindow = sortedMainWindowContextsForSessionSnapshot() - .compactMap({ $0.window ?? windowForMainWindowId($0.windowId) }) - .first { + if activateApplication, let primaryWindow = primaryRestoredWindow { primaryWindow.makeKeyAndOrderFront(nil) setActiveMainWindow(primaryWindow) NSRunningApplication.current.activate( options: [.activateAllWindows, .activateIgnoringOtherApps] )Run this read-only check to confirm all non-UI callers pass the non-activating mode:
#!/bin/bash # Description: Find restore entry points and verify socket/CLI callers do not activate the app. rg -nP -C4 '\breopenPreviousSession\s*\(' --type=swift rg -nP -C4 'session\.restore_previous|restore-session|restore_previous' --type=swift🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 3939 - 3981, reopenPreviousSession() currently always activates the app and re-queries sortedMainWindowContextsForSessionSnapshot(), which lets socket/CLI restores steal focus and can re-sort the primary restored window; change the signature to reopenPreviousSession(activateApplication: Bool = true), during the apply/create loop record the first actual NSWindow created or applied (capture primaryWindow there instead of re-calling sortedMainWindowContextsForSessionSnapshot()), call completeSessionRestoreOperation(), and only when activateApplication is true call primaryWindow.makeKeyAndOrderFront(nil), setActiveMainWindow(primaryWindow) and NSRunningApplication.current.activate(...). Ensure callers from socket/CLI pass false and update references to reopenPreviousSession() accordingly; keep uses of createMainWindow(sessionWindowSnapshot:), applySessionWindowSnapshot(...), SessionPersistencePolicy.maxWindowsPerSnapshot and setActiveMainWindow in your changes.
3949-3955:⚠️ Potential issue | 🟠 MajorClear pending startup restore state before creating manual-restore windows.
When there are no existing contexts,
createMainWindow(sessionWindowSnapshot:)registers synchronously, andregisterMainWindowcallsattemptStartupSessionRestoreIfNeededat Line 4927. IfstartupSessionSnapshotis still pending, the automatic startup restore can overwrite the manual restore snapshot being applied here.Proposed fix
let existingContexts = sortedMainWindowContextsForSessionSnapshot() + startupSessionSnapshot = nil + didAttemptStartupSessionRestore = true isApplyingSessionRestore = true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 3949 - 3955, The startup session restore can run while you synchronously create manual restore windows because createMainWindow(sessionWindowSnapshot:) -> registerMainWindow calls attemptStartupSessionRestoreIfNeeded; to prevent the automatic startup restore from overwriting the manual snapshot, clear or mark the pending startup session state (e.g. set startupSessionSnapshot = nil or flip whatever "pending" flag you use) before iterating snapshotWindows and calling createMainWindow; ensure you do this in the same scope where isApplyingSessionRestore is set so registerMainWindow/attemptStartupSessionRestoreIfNeeded will see no pending startup snapshot.Sources/RestorableAgentSession.swift (2)
197-204:⚠️ Potential issue | 🟡 MinorAvoid emitting a no-op
envprefix after filtering.If every captured env key is filtered out, the restored command still becomes
env <agent> .... It works, but it is misleading in terminal history/logs; collect safe pairs first and only prependenvwhen at least one remains.🧹 Proposed cleanup
var commandParts: [String] = [] if let env = launchCommand?.environment, !env.isEmpty { - commandParts.append("env") + var envPairs: [String] = [] for key in env.keys.sorted() { guard isSafeEnvironmentKey(key), let value = env[key] else { continue } - commandParts.append("\(key)=\(value)") + envPairs.append("\(key)=\(value)") + } + if !envPairs.isEmpty { + commandParts.append("env") + commandParts.append(contentsOf: envPairs) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 197 - 204, When building the restored shell command in RestorableAgentSession.swift (within the code that populates commandParts using launchCommand?.environment), collect filtered safe environment key=value pairs first (e.g., into a local array like safeEnvPairs by checking isSafeEnvironmentKey and non-nil value), and only append "env" and those pairs to commandParts when safeEnvPairs is not empty; this prevents emitting a no-op "env" prefix when all keys are filtered out while leaving the rest of the logic (launchCommand?.environment, isSafeEnvironmentKey, and commandParts) unchanged.
551-569:⚠️ Potential issue | 🟡 MinorMake equal-
updatedAtduplicate resolution deterministic.When two records for the same
(workspaceId, panelId)have identicalupdatedAt, the winner depends on dictionary iteration order. Include the store key/kind in the resolved tuple and use it as a stable tie-breaker.🛠️ Proposed direction
- for record in state.sessions.values { + for (recordKey, record) in state.sessions { @@ - if let existing = resolved[key], existing.updatedAt > record.updatedAt { - continue + let tieBreaker = "\(kind.rawValue):\(recordKey)" + if let existing = resolved[key] { + if existing.updatedAt > record.updatedAt { continue } + if existing.updatedAt == record.updatedAt, + existing.tieBreaker >= tieBreaker { + continue + } } - resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt) + resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt, tieBreaker: tieBreaker)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 551 - 569, The current duplicate-resolution for entries into resolved (keyed by PanelKey) only compares updatedAt, so ties are non-deterministic; modify the resolved value stored for each PanelKey to also include a stable identifier (e.g., the store key or the kind string) and update the comparison logic in the loop over state.sessions.values: when computing key = PanelKey(...), construct snapshot = SessionRestorableAgentSnapshot(...) and a storeIdentifier (for example kind or a unique storeKey from the record), then if let existing = resolved[key] { if existing.updatedAt > record.updatedAt { continue } else if existing.updatedAt == record.updatedAt { use a deterministic comparison of existing.storeIdentifier and storeIdentifier (lexicographic) to pick the winner } } finally set resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt, storeIdentifier: storeIdentifier); update all places that read resolved to expect the additional storeIdentifier field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/claude`:
- Around line 222-225: The launch arg encoding currently persists raw "$@" into
CMUX_AGENT_LAUNCH_ARGV_B64 (and thus AgentHookLaunchCommandRecord); instead,
apply the same allowlist filtering used for foreground commands before encoding:
run the incoming argv through SessionRestoreCommandSettings.isCommandAllowed (or
the existing allowlist/filter utility used by preservedClaudeArguments()) to
remove positional prompts/injected hook settings, then pass the filtered argv to
encode_launch_argv and store that result in CMUX_AGENT_LAUNCH_ARGV_B64 so only
allowed parts are written to persistent session JSON.
In `@Sources/RestorableAgentSession.swift`:
- Around line 43-73: claudeValueOptions is missing "--teammate-mode", so
preserveOptions treats the mode token as a positional and breaks resumes; add
"--teammate-mode" to the claudeValueOptions Set in RestorableAgentSession.swift
and then update claudeTeamsLaunchArguments to use two branches: (1) only prepend
"--teammate-mode auto" when the caller has NOT already provided
"--teammate-mode", and (2) always append "--settings <claudeHooksJSON>"
regardless of teammate-mode presence; reference the symbols claudeValueOptions
and claudeTeamsLaunchArguments (also mirror the same fix for the other similar
block around the second occurrence noted).
- Line 51: The "--debug" entry currently in claudeValueOptions is causing
optionWidth() to treat it as a required-value option and consume the next token;
remove "--debug" from claudeValueOptions and create a separate optional-value
option (e.g., a new optional Debug option in the same options collection) that
parses an optional category string, then update any parsing logic that uses
optionWidth()/claudeValueOptions to handle this new optional-value option so
that when "--debug" is present without a value it does not consume the next flag
(ensure the parser checks for a following token that is not a flag before
treating it as the debug value).
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 13683-13703: The code currently decodes and builds
arguments/executablePath/workingDirectory/environment then immediately
constructs AgentHookLaunchCommandRecord; instead, before creating/persisting
that record run the allowlist check via
SessionRestoreCommandSettings.isCommandAllowed(...) using the resolved
executablePath and arguments (e.g., combine executablePath and arguments from
decodeNULSeparatedBase64(...) / processArguments(for:) and pass to
isCommandAllowed), and if the command is not allowed either skip creating the
record or redact sensitive fields (arguments, workingDirectory, environment) to
nil/placeholder; ensure the same gating is applied to environment returned by
selectedAgentLaunchEnvironment(...) and any normalizedHookValue(...) results so
no unallowed argv/paths/tokens are written to session JSON.
- Around line 3184-3194: The current flow prints success after launching the app
without confirming a restored session or a working socket; modify the branch
that runs when (try? client.connect()) == nil to first wait/retry for the socket
to become available (e.g., attempt client.connect() with a short retry loop and
timeout), then call session.restore_previous() and check its return/result, only
proceeding to print jsonString(["restored": true, "launched": true]) or "OK"
when both the socket connect succeeds and session.restore_previous() indicates
success; if either step fails, return an appropriate failure path (close the
SocketClient via client.close() and print a failed/restored:false result).
Ensure you update the code paths around SocketClient, client.connect(),
client.close(), launchApp(), activateApp(), and session.restore_previous()
accordingly.
In `@Sources/AppDelegate.swift`:
- Around line 3939-3981: reopenPreviousSession() currently always activates the
app and re-queries sortedMainWindowContextsForSessionSnapshot(), which lets
socket/CLI restores steal focus and can re-sort the primary restored window;
change the signature to reopenPreviousSession(activateApplication: Bool = true),
during the apply/create loop record the first actual NSWindow created or applied
(capture primaryWindow there instead of re-calling
sortedMainWindowContextsForSessionSnapshot()), call
completeSessionRestoreOperation(), and only when activateApplication is true
call primaryWindow.makeKeyAndOrderFront(nil), setActiveMainWindow(primaryWindow)
and NSRunningApplication.current.activate(...). Ensure callers from socket/CLI
pass false and update references to reopenPreviousSession() accordingly; keep
uses of createMainWindow(sessionWindowSnapshot:),
applySessionWindowSnapshot(...), SessionPersistencePolicy.maxWindowsPerSnapshot
and setActiveMainWindow in your changes.
- Around line 3949-3955: The startup session restore can run while you
synchronously create manual restore windows because
createMainWindow(sessionWindowSnapshot:) -> registerMainWindow calls
attemptStartupSessionRestoreIfNeeded; to prevent the automatic startup restore
from overwriting the manual snapshot, clear or mark the pending startup session
state (e.g. set startupSessionSnapshot = nil or flip whatever "pending" flag you
use) before iterating snapshotWindows and calling createMainWindow; ensure you
do this in the same scope where isApplyingSessionRestore is set so
registerMainWindow/attemptStartupSessionRestoreIfNeeded will see no pending
startup snapshot.
In `@Sources/RestorableAgentSession.swift`:
- Around line 197-204: When building the restored shell command in
RestorableAgentSession.swift (within the code that populates commandParts using
launchCommand?.environment), collect filtered safe environment key=value pairs
first (e.g., into a local array like safeEnvPairs by checking
isSafeEnvironmentKey and non-nil value), and only append "env" and those pairs
to commandParts when safeEnvPairs is not empty; this prevents emitting a no-op
"env" prefix when all keys are filtered out while leaving the rest of the logic
(launchCommand?.environment, isSafeEnvironmentKey, and commandParts) unchanged.
- Around line 551-569: The current duplicate-resolution for entries into
resolved (keyed by PanelKey) only compares updatedAt, so ties are
non-deterministic; modify the resolved value stored for each PanelKey to also
include a stable identifier (e.g., the store key or the kind string) and update
the comparison logic in the loop over state.sessions.values: when computing key
= PanelKey(...), construct snapshot = SessionRestorableAgentSnapshot(...) and a
storeIdentifier (for example kind or a unique storeKey from the record), then if
let existing = resolved[key] { if existing.updatedAt > record.updatedAt {
continue } else if existing.updatedAt == record.updatedAt { use a deterministic
comparison of existing.storeIdentifier and storeIdentifier (lexicographic) to
pick the winner } } finally set resolved[key] = (snapshot: snapshot, updatedAt:
record.updatedAt, storeIdentifier: storeIdentifier); update all places that read
resolved to expect the additional storeIdentifier field.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ca4fedd-dc10-4494-9bf0-3cda830ed384
📒 Files selected for processing (8)
CLI/cmux.swiftResources/bin/claudeSources/AppDelegate.swiftSources/RestorableAgentSession.swiftSources/TabManager.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceTests.swifttests/test_claude_wrapper_hooks.py
✅ Files skipped from review due to trivial changes (1)
- cmuxTests/SessionPersistenceTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_claude_wrapper_hooks.py
- Sources/TabManager.swift
- Sources/Workspace.swift
There was a problem hiding this comment.
6 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:13711">
P2: Preserve empty argv elements when decoding `CMUX_AGENT_LAUNCH_ARGV_B64`; the current split drops them and changes the restored command.</violation>
<violation number="2" location="CLI/cmux.swift:13719">
P2: Persist the wrapper-resolution env vars here. Without `PATH`/`CMUX_CUSTOM_CLAUDE_PATH`, restored `claude-teams` and `omo` sessions can launch a different binary or fail to launch.</violation>
</file>
<file name="Sources/RestorableAgentSession.swift">
<violation number="1" location="Sources/RestorableAgentSession.swift:51">
P2: `--debug` accepts an **optional** category filter (e.g., `claude --debug "api,mcp"`) per the Claude Code CLI reference, but listing it in `claudeValueOptions` causes `optionWidth` to always return 2 and consume the next token. When `--debug` is followed by another flag (e.g., `--debug --model sonnet`), the parser swallows `--model` as the value of `--debug`, silently losing the model flag from the resume command. Move `--debug` to a separate optional-value set, or add a heuristic that skips consumption when the next token starts with `--`.</violation>
<violation number="2" location="Sources/RestorableAgentSession.swift:71">
P1: Add `--teammate-mode` to `claudeValueOptions`. This option takes a space-separated value (`auto`, `in-process`, `tmux`), but since it's not in `claudeValueOptions`, `optionWidth` returns 1. The value token (e.g., `auto`) is then treated as a positional argument, triggering a `break` in the parser loop and silently discarding all subsequent flags from the resume command.</violation>
<violation number="3" location="Sources/RestorableAgentSession.swift:410">
P1: Dropped value-options (`--resume`, `--session-id`, `--tmux`, `--worktree`/`-w`, `--from-pr`, `--fork-session`) are not in `claudeValueOptions`, so when `shouldDropOption` matches and calls `optionWidth`, it returns 1 instead of 2. The unconsumed value token is treated as a positional argument, triggering `break` and silently discarding all subsequent flags. For example, `--tmux main --permission-mode auto` drops `--tmux`, then `main` causes a break, losing `--permission-mode auto`. The test `testClaudeTeamsResumeCommandPreservesRemoteControlLauncher` would fail for this reason.</violation>
<violation number="4" location="Sources/RestorableAgentSession.swift:472">
P1: Preserve `NODE_OPTIONS` in the restored environment. The new whitelist drops the saved Node runtime flags, so resumed agent sessions can relaunch with a different runtime than the original session.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/Workspace.swift (1)
712-718:⚠️ Potential issue | 🟠 MajorGuard restored agent resume commands with the saved cwd.
resumeCommandis still injected directly into a fresh shell. If shell rc filescdelsewhere before this input runs, the agent resumes in the wrong directory. Use a cwd-guarded helper onSessionRestorableAgentSnapshotbeforesendInputWhenReady.Proposed call-site shape
- if let resumeCommand = snapshot.terminal?.agent?.resumeCommand { + if let restorableAgent = snapshot.terminal?.agent { + let resumeCommand = restorableAgent.resumeCommandWithCwd sendInputWhenReady( resumeCommand + "\n", to: terminalPanel, reason: .restoredAgentResume ) }Based on learnings: Session resume behavior should generate commands with a guaranteed cwd guard, using a helper that produces
cd <shell-quoted cwd> && <resumeCommand>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 712 - 718, The resumeCommand is being injected directly into a fresh shell (snapshot.terminal?.agent?.resumeCommand) which can run in the wrong cwd; update the call-site to use a cwd-guarded helper on the SessionRestorableAgentSnapshot (e.g. a method like guardedResumeCommand(forShell:) or resumeCommandWithCwdGuard()) that returns a string of the form "cd <shell-quoted cwd> && <resumeCommand>", then pass that guarded string into sendInputWhenReady(to: terminalPanel, reason: .restoredAgentResume) instead of the raw resumeCommand so the agent always resumes in the saved working directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/Workspace.swift`:
- Around line 712-718: The resumeCommand is being injected directly into a fresh
shell (snapshot.terminal?.agent?.resumeCommand) which can run in the wrong cwd;
update the call-site to use a cwd-guarded helper on the
SessionRestorableAgentSnapshot (e.g. a method like
guardedResumeCommand(forShell:) or resumeCommandWithCwdGuard()) that returns a
string of the form "cd <shell-quoted cwd> && <resumeCommand>", then pass that
guarded string into sendInputWhenReady(to: terminalPanel, reason:
.restoredAgentResume) instead of the raw resumeCommand so the agent always
resumes in the saved working directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 511a0de3-18ac-4825-baa3-1ee188c7d4c4
📒 Files selected for processing (2)
Sources/Workspace.swiftcmuxTests/SessionPersistenceTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- cmuxTests/SessionPersistenceTests.swift
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
Sources/RestorableAgentSession.swift (4)
198-205:⚠️ Potential issue | 🟡 MinorAvoid emitting
envwhen every captured key is filtered out.Line 200 appends
envbefore the allow-list check. If the captured environment contains only unsafe keys, the generated command becomesenv <agent> ...; it works, but it is noisy and misleading in restored terminal history.🛠️ Proposed fix
var commandParts: [String] = [] if let env = launchCommand?.environment, !env.isEmpty { - commandParts.append("env") + var envPairs: [String] = [] for key in env.keys.sorted() { guard isSafeEnvironmentKey(key), let value = env[key] else { continue } - commandParts.append("\(key)=\(value)") + envPairs.append("\(key)=\(value)") + } + if !envPairs.isEmpty { + commandParts.append("env") + commandParts.append(contentsOf: envPairs) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 198 - 205, The code currently appends "env" to commandParts before checking allowed keys, causing a stray "env" when all keys are filtered; change the logic in the RestorableAgentSession code that builds commandParts (the block referencing launchCommand?.environment and isSafeEnvironmentKey) to first collect allowed environment entries into a temporary array (e.g., envParts) by iterating env.keys.sorted() and pushing "\(key)=\(value)" only for keys passing isSafeEnvironmentKey, and only if that temporary array is non-empty append "env" followed by those entries to commandParts.
567-570:⚠️ Potential issue | 🟡 MinorMake equal-
updatedAtconflict resolution deterministic.When multiple records for the same
(workspaceId, panelId)have identical timestamps, the winner still depends on dictionary iteration order within a hook store. Prefer an explicit tie-breaker, or use a stable policy such as keeping the first resolved record with>=.🛠️ Minimal deterministic policy
- if let existing = resolved[key], existing.updatedAt > record.updatedAt { + if let existing = resolved[key], existing.updatedAt >= record.updatedAt { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 567 - 570, The current conflict resolution in RestorableAgentSession.swift (variables: resolved, key, existing, record.updatedAt, snapshot) is non-deterministic when timestamps are equal; change the comparison logic so ties are deterministic by keeping the first-resolved record—replace the strict greater-than check (existing.updatedAt > record.updatedAt) with a greater-than-or-equal check (existing.updatedAt >= record.updatedAt) or implement an explicit tie-breaker policy so equal updatedAt values do not depend on dictionary iteration order; ensure the resolved[key] assignment remains unchanged (resolved[key] = (snapshot: snapshot, updatedAt: record.updatedAt)) so the first winner is preserved.
43-73:⚠️ Potential issue | 🟡 MinorHandle
--debugas an optional-value Claude option.Line 51 still makes
--debugconsume the next token unconditionally. For launches likeclaude --debug --model sonnet,--modelis treated as the debug value andsonnetbecomes a positional, so the reconstructed resume command drops the model value and can be malformed.🛠️ Proposed fix sketch
+ private static let claudeOptionalValueOptions: Set<String> = [ + "--debug" + ] + private static let claudeValueOptions: Set<String> = [ @@ - "--debug", "--debug-file", @@ - valueOptions: claudeValueOptions, + valueOptions: claudeValueOptions, + optionalValueOptions: claudeOptionalValueOptions, @@ private static func optionWidth( _ args: [String], index: Int, valueOptions: Set<String>, + optionalValueOptions: Set<String> = [], variadicOptions: Set<String> ) -> Int { let arg = args[index] if arg.contains("=") { return 1 } + if optionalValueOptions.contains(arg) { + guard index + 1 < args.count, !args[index + 1].hasPrefix("-") else { + return 1 + } + return 2 + } guard valueOptions.contains(arg), index + 1 < args.count else { return 1 }Claude Code CLI reference --debug optional category filterAlso applies to: 420-454
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 43 - 73, The claudeValueOptions set currently forces "--debug" to always consume the next token, causing "--model" to be mis-parsed; remove "--debug" from the claudeValueOptions Set<String> (in RestorableAgentSession) and update the parsing logic that uses claudeValueOptions so "--debug" is handled as an optional-value flag: if the next token exists and does not start with "-" treat it as the debug value, otherwise treat "--debug" as a boolean flag that does not consume the following token. Ensure the same change is applied wherever claudeValueOptions is referenced so resume command reconstruction preserves the correct --model and positional arguments.
43-73:⚠️ Potential issue | 🟠 MajorPreserve
--teammate-modeas a value option.Claude Teams launches can include
--teammate-mode auto|manual. Since Line 72 omits--teammate-mode, Line 231 preserves only the flag, then treatsauto/manualas a positional and stops parsing, producing an incomplete resume command.🛠️ Proposed fix
"--setting-sources", "--settings", "--system-prompt", + "--teammate-mode", "--tools"Based on learnings: "In CLI/cmux.swift,
claudeTeamsLaunchArgumentsmust use two independent branches: (1) prepend--teammate-mode autoonly when the caller has NOT already supplied--teammate-mode; (2) append--settings <claudeHooksJSON>unconditionally..."Also applies to: 221-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 43 - 73, The parser is treating the teammate mode token as a positional because "--teammate-mode" is missing from the claudeValueOptions set; add "--teammate-mode" to the claudeValueOptions Set<String> so the following token (auto|manual) is consumed as the option's value, and update the resume/launch assembly logic in claudeTeamsLaunchArguments (and the code path that preserves flags around where flags are filtered/preserved) to use two branches: (1) if caller did NOT supply "--teammate-mode", prepend "--teammate-mode auto" (or the desired default), and (2) always append "--settings <claudeHooksJSON>" unconditionally—this ensures existing caller-provided "--teammate-mode <value>" is preserved and parsing doesn't stop on the mode token.CLI/cmux.swift (1)
3215-3225:⚠️ Potential issue | 🟠 MajorDon’t report restore success before the launched app is connectable.
This path prints
OK/{"restored": true}immediately afteropen -a cmux, without waiting for the socket or invokingsession.restore_previous. If launch fails afteropenreturns, or startup restore is disabled/broken, the CLI still reports a successful restore.Proposed fix: wait for the launched app and use the same restore RPC
let client = SocketClient(path: socketPath) if (try? client.connect()) == nil { client.close() try launchApp() - try activateApp() + let launchedClient = try SocketClient.waitForConnectableSocket(path: socketPath, timeout: 10) + defer { launchedClient.close() } + try authenticateClientIfNeeded( + launchedClient, + explicitPassword: explicitPassword, + socketPath: socketPath + ) + var response = try launchedClient.sendV2(method: "session.restore_previous") + response["launched"] = true + try activateApp() if jsonOutput { - print(jsonString(["restored": true, "launched": true])) + print(jsonString(response)) } else { print("OK") } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3215 - 3225, The current branch returns success immediately after calling launchApp()/activateApp() without verifying the launched app is reachable; modify the flow so that after calling launchApp() and activateApp() you attempt to connect the SocketClient (using the existing SocketClient(path: socketPath) and its connect() method) in a retry loop with a short timeout; once connect() succeeds, invoke the same restore RPC (session.restore_previous) over that connected client to confirm restore completed, only then print the JSON/"OK" success and return; ensure you close the client on failure and surface errors if the socket never becomes connectable or the restore RPC fails.
🧹 Nitpick comments (2)
Sources/RestorableAgentSession.swift (1)
585-587: Remove the redundant memberwise initializer.SwiftLint is right here: this private initializer is equivalent to the synthesized one and can be removed.
♻️ Proposed cleanup
- - private init(snapshotsByPanel: [PanelKey: SessionRestorableAgentSnapshot]) { - self.snapshotsByPanel = snapshotsByPanel - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RestorableAgentSession.swift` around lines 585 - 587, The private initializer private init(snapshotsByPanel: [PanelKey: SessionRestorableAgentSnapshot]) in RestorableAgentSession is redundant because it duplicates the compiler-synthesized memberwise initializer; remove this initializer declaration from Sources/RestorableAgentSession.swift and run the build/lint to confirm no call sites depend explicitly on it (there should be none since the synthesized one will be used).cmuxTests/SessionPersistenceTests.swift (1)
1091-1407: Consider moving agent-resume tests out ofSocketListenerAcceptPolicyTests.These new tests exercise
SessionRestorableAgentSnapshot,WorkspacePendingTerminalInputPolicy, andRestorableAgentSessionIndex, not socket accept policy. A dedicatedRestorableAgentSessionTestsclass would keep failures easier to navigate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/SessionPersistenceTests.swift` around lines 1091 - 1407, Move the new session-resume related tests out of SocketListenerAcceptPolicyTests into a dedicated test class (e.g. RestorableAgentSessionTests) so they live with related code; create a new XCTestCase subclass and relocate the methods testClaudeResumeCommandPreservesLaunchFlagsAndDropsInjectedHookSettings, testCodexResumeCommandPreservesFlagsAndDropsOriginalPrompt, testClaudeTeamsResumeCommandPreservesRemoteControlLauncher, testOpenCodeWrapperResumeCommandAndUnsupportedOhMyLaunchers, testRestoredAgentResumeInputDoesNotExpireForInactiveWorkspaceTerminals, testNonInteractiveAgentLaunchesAreNotAutoRestored, and testRestorableAgentIndexLoadsLaunchCommandFromHookStore into that class, update any imports/fixtures as needed, and remove them from SocketListenerAcceptPolicyTests (or the original test class) so failures and maintenance target SessionRestorableAgentSnapshot, WorkspacePendingTerminalInputPolicy, and RestorableAgentSessionIndex are easier to navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 416-418: The current code assigns raw argv to record.launchCommand
from agentLaunchCommandFromEnvironment and persists it (via upsert) without
checking or redacting sensitive content; change the logic to check the candidate
command against SessionRestoreCommandSettings.isCommandAllowed(...) and only set
record.launchCommand if allowed, otherwise either redact it using the existing
restore-command redaction path or omit it entirely; update the same pattern
where launchCommand is set/persisted (including the other occurrence around
lines 13722-13748) so unallowed commands are never written to session JSON.
- Around line 13687-13692: The call to copyBytes(to:from:) on
UnsafeRawBufferPointer is invalid; update the inner closure that uses
buffer.withUnsafeBytes to either (a) call argcBytes.copyBytes(from:
bufferBytes.prefix(MemoryLayout<Int32>.size)) using the
UnsafeMutableRawBufferPointer API, or (b) perform an explicit byte-by-byte copy
loop from bufferBytes to argcBytes for 0..<MemoryLayout<Int32>.size; modify the
closure that references argc, buffer, withUnsafeMutableBytes, withUnsafeBytes
and MemoryLayout<Int32>.size accordingly so the raw-buffer copy uses a supported
API or manual copy.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 3215-3225: The current branch returns success immediately after
calling launchApp()/activateApp() without verifying the launched app is
reachable; modify the flow so that after calling launchApp() and activateApp()
you attempt to connect the SocketClient (using the existing SocketClient(path:
socketPath) and its connect() method) in a retry loop with a short timeout; once
connect() succeeds, invoke the same restore RPC (session.restore_previous) over
that connected client to confirm restore completed, only then print the
JSON/"OK" success and return; ensure you close the client on failure and surface
errors if the socket never becomes connectable or the restore RPC fails.
In `@Sources/RestorableAgentSession.swift`:
- Around line 198-205: The code currently appends "env" to commandParts before
checking allowed keys, causing a stray "env" when all keys are filtered; change
the logic in the RestorableAgentSession code that builds commandParts (the block
referencing launchCommand?.environment and isSafeEnvironmentKey) to first
collect allowed environment entries into a temporary array (e.g., envParts) by
iterating env.keys.sorted() and pushing "\(key)=\(value)" only for keys passing
isSafeEnvironmentKey, and only if that temporary array is non-empty append "env"
followed by those entries to commandParts.
- Around line 567-570: The current conflict resolution in
RestorableAgentSession.swift (variables: resolved, key, existing,
record.updatedAt, snapshot) is non-deterministic when timestamps are equal;
change the comparison logic so ties are deterministic by keeping the
first-resolved record—replace the strict greater-than check (existing.updatedAt
> record.updatedAt) with a greater-than-or-equal check (existing.updatedAt >=
record.updatedAt) or implement an explicit tie-breaker policy so equal updatedAt
values do not depend on dictionary iteration order; ensure the resolved[key]
assignment remains unchanged (resolved[key] = (snapshot: snapshot, updatedAt:
record.updatedAt)) so the first winner is preserved.
- Around line 43-73: The claudeValueOptions set currently forces "--debug" to
always consume the next token, causing "--model" to be mis-parsed; remove
"--debug" from the claudeValueOptions Set<String> (in RestorableAgentSession)
and update the parsing logic that uses claudeValueOptions so "--debug" is
handled as an optional-value flag: if the next token exists and does not start
with "-" treat it as the debug value, otherwise treat "--debug" as a boolean
flag that does not consume the following token. Ensure the same change is
applied wherever claudeValueOptions is referenced so resume command
reconstruction preserves the correct --model and positional arguments.
- Around line 43-73: The parser is treating the teammate mode token as a
positional because "--teammate-mode" is missing from the claudeValueOptions set;
add "--teammate-mode" to the claudeValueOptions Set<String> so the following
token (auto|manual) is consumed as the option's value, and update the
resume/launch assembly logic in claudeTeamsLaunchArguments (and the code path
that preserves flags around where flags are filtered/preserved) to use two
branches: (1) if caller did NOT supply "--teammate-mode", prepend
"--teammate-mode auto" (or the desired default), and (2) always append
"--settings <claudeHooksJSON>" unconditionally—this ensures existing
caller-provided "--teammate-mode <value>" is preserved and parsing doesn't stop
on the mode token.
---
Nitpick comments:
In `@cmuxTests/SessionPersistenceTests.swift`:
- Around line 1091-1407: Move the new session-resume related tests out of
SocketListenerAcceptPolicyTests into a dedicated test class (e.g.
RestorableAgentSessionTests) so they live with related code; create a new
XCTestCase subclass and relocate the methods
testClaudeResumeCommandPreservesLaunchFlagsAndDropsInjectedHookSettings,
testCodexResumeCommandPreservesFlagsAndDropsOriginalPrompt,
testClaudeTeamsResumeCommandPreservesRemoteControlLauncher,
testOpenCodeWrapperResumeCommandAndUnsupportedOhMyLaunchers,
testRestoredAgentResumeInputDoesNotExpireForInactiveWorkspaceTerminals,
testNonInteractiveAgentLaunchesAreNotAutoRestored, and
testRestorableAgentIndexLoadsLaunchCommandFromHookStore into that class, update
any imports/fixtures as needed, and remove them from
SocketListenerAcceptPolicyTests (or the original test class) so failures and
maintenance target SessionRestorableAgentSnapshot,
WorkspacePendingTerminalInputPolicy, and RestorableAgentSessionIndex are easier
to navigate.
In `@Sources/RestorableAgentSession.swift`:
- Around line 585-587: The private initializer private init(snapshotsByPanel:
[PanelKey: SessionRestorableAgentSnapshot]) in RestorableAgentSession is
redundant because it duplicates the compiler-synthesized memberwise initializer;
remove this initializer declaration from Sources/RestorableAgentSession.swift
and run the build/lint to confirm no call sites depend explicitly on it (there
should be none since the synthesized one will be used).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9087243e-613e-4724-81d3-abf3265ecb54
📒 Files selected for processing (3)
CLI/cmux.swiftSources/RestorableAgentSession.swiftcmuxTests/SessionPersistenceTests.swift
lawrencecchen
force-pushed
the
issue-2923-reopen-sessions
branch
from
c7fd046 to
d41d18d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9cd3d82. Configure here.
Contributor
|
Greptile's stale P1 summary at #2978 (comment) is already addressed in the current branch. The socket handler calls cmux/Sources/TerminalController.swift Line 7028 in 450b16c shouldActivate parameter in cmux/Sources/AppDelegate.swift Line 2311 in 450b16c |
|
this is going to be huge, so excited for next version release @lawrencecchen |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
File > Reopen Previous Session,⌘⇧O, orcmux restore-sessionScope
Testing
python3 -m py_compile tests/test_session_relaunch_resumes_agent_sessions.pytests/test_restore_session_relaunches_codex_resume.py./scripts/reload.sh --tag issue-2923-reopen-sessionsbut localxcodebuildstalled in the Xcode environment before producing a compile resultNote
Medium Risk
Touches session persistence/restore flow and injects auto-resume input into restored terminals, plus adds new CLI/RPC and hook/plugin integrations; mistakes could lead to incorrect restores or unintended command execution on relaunch.
Overview
Adds a manual session restore capability (“Reopen Previous Session”) exposed via app menu/shortcut/command palette, new RPC
session.restore_previous, and CLIcmux restore-session, backed by a cached*-previoussnapshot file to restore even after a blank relaunch.Extends session snapshots to include restorable agent metadata for terminal panels and uses it during restore to auto-inject a bounded resume command (and to suppress scrollback replay when resuming). This introduces a new
RestorableAgentSessionIndexbuilt from hook state files and threads it into autosave fingerprinting/snapshot building so agent resume state participates in persistence.Expands the hook/CLI surface to support OpenCode (install/uninstall hooks via an installed plugin file, plus
opencode-hookdispatch) and captures/sanitizes agent launch context (argv/cwd/env,NODE_OPTIONSnormalization) from env or live process args for safer resume reconstruction; README and localizations are updated accordingly.Reviewed by Cursor Bugbot for commit 450b16c. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds “Reopen Previous Session” with auto/manual restore and reliable auto-resume for Claude Code, Codex, and OpenCode. Hardened restore flow for issue 2923 to survive blank relaunches, keep resume input within terminal limits, and prune stale agent state.
New Features
session.restore_previous, CLIcmux restore-session; uses a cached “previous snapshot” after a blank relaunch.NODE_OPTIONS, queues resumes for lazy terminals, embeds agent data in terminal snapshots, and installs/uninstalls the OpenCode session plugin.Bug Fixes
Written for commit 450b16c. Summary will update on new commits.
Summary by CodeRabbit
New Features
cmux restore-session(optional JSON output) and new OpenCode hook subcommands (install/uninstall).Documentation
Tests
Localization