From f15bf28a80c231ebb7e5e60fd9b95577af4b73f4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:29:53 -0700 Subject: [PATCH 1/5] Add failing coverage: corrupt session snapshot must not destroy restore backup A corrupt primary session snapshot at startup currently makes syncManualRestoreSnapshotCache delete session--previous.json (the restore-session backup) and the app silently starts fresh. These tests pin the desired behavior: keep the backup and recover startup restore from it. Includes tests/test_session_restore_stress_kill_cycles.py, an end-to-end stress harness covering repeated clean relaunches, SIGKILL relaunch, and corrupt-snapshot recovery for tracked Claude/Codex/OpenCode sessions. SessionPersistenceStore.syncManualRestoreSnapshotCache and the new loadStartupSnapshot gain injectable bundle/app-support parameters (behavior unchanged in this commit) so the regression tests can run against temp paths. --- Sources/AppDelegate.swift | 2 +- Sources/SessionPersistence.swift | 31 +- cmuxTests/SessionPersistenceTests.swift | 145 ++++++ ...test_session_restore_stress_kill_cycles.py | 411 ++++++++++++++++++ 4 files changed, 583 insertions(+), 6 deletions(-) create mode 100644 tests/test_session_restore_stress_kill_cycles.py diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 4bc99bba54c2..a53c696ca16a 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -3071,7 +3071,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent Self.removeLegacyPersistedWindowGeometry() SessionPersistenceStore.syncManualRestoreSnapshotCache() guard SessionRestorePolicy.shouldAttemptRestore() else { return } - startupSessionSnapshot = SessionPersistenceStore.load() + startupSessionSnapshot = SessionPersistenceStore.loadStartupSnapshot() } private func persistedWindowGeometry(defaults: UserDefaults = .standard) -> PersistedWindowGeometry? { diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index 6b8ffa0ef161..adf5ffc1c946 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -1920,13 +1920,34 @@ enum SessionPersistenceStore { return load(fileURL: fileURL) } - static func syncManualRestoreSnapshotCache() { - guard let fileURL = manualRestoreSnapshotFileURL() else { return } - guard let snapshot = load() else { - removeSnapshot(fileURL: fileURL) + static func syncManualRestoreSnapshotCache( + bundleIdentifier: String? = Bundle.main.bundleIdentifier, + appSupportDirectory: URL? = nil + ) { + guard let backupURL = manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: appSupportDirectory + ) else { return } + guard let primaryURL = defaultSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: appSupportDirectory + ) else { return } + guard let snapshot = load(fileURL: primaryURL) else { + removeSnapshot(fileURL: backupURL) return } - _ = save(snapshot, fileURL: fileURL) + _ = save(snapshot, fileURL: backupURL) + } + + static func loadStartupSnapshot( + bundleIdentifier: String? = Bundle.main.bundleIdentifier, + appSupportDirectory: URL? = nil + ) -> AppSessionSnapshot? { + guard let primaryURL = defaultSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: appSupportDirectory + ) else { return nil } + return load(fileURL: primaryURL) } static func defaultSnapshotFileURL( diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index 305736945c3e..2b1544f8700d 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -238,6 +238,151 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(loaded.windows.first?.sidebar.width, 321) } + func testSyncManualRestoreCachePreservesBackupWhenPrimarySnapshotIsCorrupt() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" + let primaryURL = try XCTUnwrap( + SessionPersistenceStore.defaultSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + let backupURL = try XCTUnwrap( + SessionPersistenceStore.manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + + XCTAssertTrue( + SessionPersistenceStore.save( + makeSnapshot(version: SessionSnapshotSchema.currentVersion), + fileURL: backupURL + ) + ) + try FileManager.default.createDirectory( + at: primaryURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try Data("{\"version\": 9999, \"windows\": [truncated-mid-w".utf8).write(to: primaryURL) + + SessionPersistenceStore.syncManualRestoreSnapshotCache( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + + XCTAssertNotNil( + SessionPersistenceStore.load(fileURL: backupURL), + "A corrupt primary snapshot must not destroy the restore-session backup" + ) + } + + func testSyncManualRestoreCacheRemovesBackupWhenPrimarySnapshotIsMissing() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" + let backupURL = try XCTUnwrap( + SessionPersistenceStore.manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + + XCTAssertTrue( + SessionPersistenceStore.save( + makeSnapshot(version: SessionSnapshotSchema.currentVersion), + fileURL: backupURL + ) + ) + + SessionPersistenceStore.syncManualRestoreSnapshotCache( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + + XCTAssertNil( + SessionPersistenceStore.load(fileURL: backupURL), + "A genuinely absent primary snapshot still clears the stale backup" + ) + } + + func testStartupSnapshotLoadRecoversFromBackupWhenPrimarySnapshotIsCorrupt() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" + let primaryURL = try XCTUnwrap( + SessionPersistenceStore.defaultSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + let backupURL = try XCTUnwrap( + SessionPersistenceStore.manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + + var backupSnapshot = makeSnapshot(version: SessionSnapshotSchema.currentVersion) + backupSnapshot.windows[0].sidebar.width = 321 + XCTAssertTrue(SessionPersistenceStore.save(backupSnapshot, fileURL: backupURL)) + try FileManager.default.createDirectory( + at: primaryURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try Data("not a session snapshot".utf8).write(to: primaryURL) + + let loaded = SessionPersistenceStore.loadStartupSnapshot( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + + XCTAssertEqual( + loaded?.windows.first?.sidebar.width, + 321, + "Startup restore must fall back to the backup snapshot when the primary is corrupt" + ) + } + + func testStartupSnapshotLoadReturnsNilWhenPrimarySnapshotIsMissing() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" + let backupURL = try XCTUnwrap( + SessionPersistenceStore.manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ) + XCTAssertTrue( + SessionPersistenceStore.save( + makeSnapshot(version: SessionSnapshotSchema.currentVersion), + fileURL: backupURL + ) + ) + + XCTAssertNil( + SessionPersistenceStore.loadStartupSnapshot( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ), + "A clean start without a primary snapshot must not resurrect the backup" + ) + } + func testSaveAndLoadRoundTripPreservesWorkspaceCustomColor() { let tempDir = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) diff --git a/tests/test_session_restore_stress_kill_cycles.py b/tests/test_session_restore_stress_kill_cycles.py new file mode 100644 index 000000000000..716e5564eafe --- /dev/null +++ b/tests/test_session_restore_stress_kill_cycles.py @@ -0,0 +1,411 @@ +#!/usr/bin/env python3 +""" +Stress: tracked agent sessions must survive repeated kill/reopen cycles. + +Phases: +1) Seed six workspaces with tracked Claude/Codex/OpenCode sessions. The fake + agents keep running (exec sleep) so the shell stays in command-running + state and every later snapshot records wasAgentRunning=true. +2) Clean quit -> relaunch: all six sessions auto-resume from the persisted + snapshot (hook state files are deleted before relaunch to prove it). +3) Second clean quit -> relaunch: the re-saved snapshot still resumes all six. +4) SIGKILL the app after the autosave window -> relaunch: the autosaved + snapshot still resumes all six. +5) Clean quit, then corrupt the primary session snapshot -> relaunch: cmux + must recover the session from the -previous backup snapshot instead of + silently starting fresh, and the backup file must survive the relaunch so + `cmux restore-session` keeps working. +""" + +from __future__ import annotations + +import json +import os +import plistlib +import re +import signal +import socket +import subprocess +import tempfile +import time +from pathlib import Path + +from cmux import cmux + +SESSION_SPECS = [ + ("claude", "claude-stress-0", "CMUX_FAKE_CLAUDE_RESUME:--resume claude-stress-0"), + ("claude", "claude-stress-1", "CMUX_FAKE_CLAUDE_RESUME:--resume claude-stress-1"), + ("codex", "codex-stress-0", "CMUX_FAKE_CODEX_RESUME:resume codex-stress-0"), + ("codex", "codex-stress-1", "CMUX_FAKE_CODEX_RESUME:resume codex-stress-1"), + ("opencode", "opencode-stress-0", "CMUX_FAKE_OPENCODE_RESUME:--session opencode-stress-0"), + ("opencode", "opencode-stress-1", "CMUX_FAKE_OPENCODE_RESUME:--session opencode-stress-1"), +] + + +def _bundle_id(app_path: Path) -> str: + info_path = app_path / "Contents" / "Info.plist" + if not info_path.exists(): + raise RuntimeError(f"Missing Info.plist at {info_path}") + with info_path.open("rb") as f: + info = plistlib.load(f) + bundle_id = str(info.get("CFBundleIdentifier", "")).strip() + if not bundle_id: + raise RuntimeError("Missing CFBundleIdentifier") + return bundle_id + + +def _snapshot_path(bundle_id: str, suffix: str = "") -> Path: + safe_bundle = re.sub(r"[^A-Za-z0-9._-]", "_", bundle_id) + return Path.home() / "Library/Application Support/cmux" / f"session-{safe_bundle}{suffix}.json" + + +def _socket_reachable(socket_path: Path) -> bool: + if not socket_path.exists(): + return False + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + sock.settimeout(0.3) + sock.connect(str(socket_path)) + sock.sendall(b"ping\n") + data = sock.recv(1024) + return b"PONG" in data + except OSError: + return False + finally: + sock.close() + + +def _wait_for_socket(socket_path: Path, timeout: float = 20.0) -> None: + deadline = time.time() + timeout + while time.time() < deadline: + if _socket_reachable(socket_path): + return + time.sleep(0.2) + raise RuntimeError(f"Socket did not become reachable: {socket_path}") + + +def _wait_for_socket_closed(socket_path: Path, timeout: float = 20.0) -> None: + deadline = time.time() + timeout + while time.time() < deadline: + if not _socket_reachable(socket_path): + return + time.sleep(0.2) + raise RuntimeError(f"Socket still reachable after quit: {socket_path}") + + +def _app_pids(app_path: Path) -> list[int]: + exe = app_path / "Contents" / "MacOS" / "cmux DEV" + result = subprocess.run(["pgrep", "-f", str(exe)], capture_output=True, text=True) + return [int(line) for line in result.stdout.split() if line.strip().isdigit()] + + +def _kill_existing(app_path: Path) -> None: + exe = app_path / "Contents" / "MacOS" / "cmux DEV" + subprocess.run(["pkill", "-f", str(exe)], capture_output=True, text=True) + time.sleep(1.0) + + +def _launch(app_path: Path, socket_path: Path, env_overrides: dict[str, str] | None = None) -> None: + try: + socket_path.unlink() + except FileNotFoundError: + pass + + command = ["open", "-na", str(app_path)] + full_env = dict(env_overrides or {}) + full_env["CMUX_SOCKET_PATH"] = str(socket_path) + full_env["CMUX_ALLOW_SOCKET_OVERRIDE"] = "1" + for key, value in full_env.items(): + command.extend(["--env", f"{key}={value}"]) + subprocess.run(command, check=True) + _wait_for_socket(socket_path) + time.sleep(1.5) + + +def _quit(bundle_id: str, socket_path: Path) -> None: + subprocess.run( + ["osascript", "-e", f'tell application id "{bundle_id}" to quit'], + capture_output=True, + text=True, + check=True, + ) + _wait_for_socket_closed(socket_path) + try: + socket_path.unlink() + except FileNotFoundError: + pass + time.sleep(0.8) + + +def _force_kill(app_path: Path, socket_path: Path) -> None: + pids = _app_pids(app_path) + if not pids: + raise RuntimeError("expected a running app to SIGKILL") + for pid in pids: + try: + os.kill(pid, signal.SIGKILL) + except ProcessLookupError: + pass + _wait_for_socket_closed(socket_path) + try: + socket_path.unlink() + except FileNotFoundError: + pass + time.sleep(0.8) + + +def _connect(socket_path: Path) -> cmux: + client = cmux(socket_path=str(socket_path)) + client.connect() + if not client.ping(): + raise RuntimeError("ping failed") + return client + + +def _read_scrollback(client: cmux) -> str: + return client._send_command("read_screen --scrollback") + + +def _wait_for_condition(timeout: float, predicate) -> bool: + deadline = time.time() + timeout + while time.time() < deadline: + if predicate(): + return True + time.sleep(0.3) + return False + + +def _write_fake_agent(fake_bin_dir: Path, binary_name: str, prefix: str) -> None: + fake_bin_dir.mkdir(parents=True, exist_ok=True) + fake_binary = fake_bin_dir / binary_name + # Keep running so snapshots record the agent as live (wasAgentRunning=true) + # and later relaunches keep auto-resuming. + fake_binary.write_text( + "#!/bin/sh\n" + f"printf '{prefix}:%s\\n' \"$*\"\n" + "exec sleep 86400\n", + encoding="utf-8", + ) + fake_binary.chmod(0o755) + + +def _write_hook_state( + path: Path, + sessions: list[dict], +) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + payload = {"version": 1, "sessions": {entry["sessionId"]: entry for entry in sessions}} + path.write_text(json.dumps(payload), encoding="utf-8") + + +def _hook_session_entry( + session_id: str, + workspace_id: str, + surface_id: str, + cwd: str, + launcher: str, + executable_path: Path, +) -> dict: + return { + "sessionId": session_id, + "workspaceId": workspace_id, + "surfaceId": surface_id, + "cwd": cwd, + "launchCommand": { + "launcher": launcher, + "executablePath": str(executable_path), + "arguments": [str(executable_path)], + "workingDirectory": cwd, + "capturedAt": time.time(), + "source": "test", + }, + "updatedAt": time.time(), + } + + +def _collect_all_scrollbacks(client: cmux) -> str: + chunks: list[str] = [] + workspaces = client.list_workspaces() + for index in range(len(workspaces)): + client.select_workspace(index) + chunks.append(_read_scrollback(client)) + return "\n".join(chunks) + + +def _assert_all_sessions_resumed( + client: cmux, + phase: str, + failures: list[str], + timeout: float = 30.0, +) -> None: + expected_markers = [marker for (_, _, marker) in SESSION_SPECS] + + def all_present() -> bool: + if len(client.list_workspaces()) < len(SESSION_SPECS): + return False + combined = _collect_all_scrollbacks(client) + return all(marker in combined for marker in expected_markers) + + if _wait_for_condition(timeout, all_present): + return + combined = _collect_all_scrollbacks(client) + missing = [marker for marker in expected_markers if marker not in combined] + workspace_count = len(client.list_workspaces()) + failures.append( + f"{phase}: {len(missing)}/{len(expected_markers)} sessions did not resume " + f"(workspaces={workspace_count}); missing markers: {missing}" + ) + + +def main() -> int: + app_path_str = os.environ.get("CMUX_APP_PATH", "").strip() + if not app_path_str: + print("SKIP: set CMUX_APP_PATH to a built cmux DEV .app path") + return 0 + app_path = Path(app_path_str) + if not app_path.exists(): + print(f"SKIP: CMUX_APP_PATH does not exist: {app_path}") + return 0 + + bundle_id = _bundle_id(app_path) + socket_path = Path(f"/tmp/cmux-restore-stress-{bundle_id.replace('.', '-')}.sock") + snapshot = _snapshot_path(bundle_id) + previous_snapshot = _snapshot_path(bundle_id, suffix="-previous") + + failures: list[str] = [] + + with tempfile.TemporaryDirectory(prefix="cmux-restore-stress-") as td: + fake_bin_dir = Path(td) / "bin" + hook_state_dir = Path(td) / "hook-state" + hook_state_files = { + launcher: hook_state_dir / f"{launcher}-hook-sessions.json" + for launcher in {launcher for (launcher, _, _) in SESSION_SPECS} + } + _write_fake_agent(fake_bin_dir, "claude", "CMUX_FAKE_CLAUDE_RESUME") + _write_fake_agent(fake_bin_dir, "codex", "CMUX_FAKE_CODEX_RESUME") + _write_fake_agent(fake_bin_dir, "opencode", "CMUX_FAKE_OPENCODE_RESUME") + launch_path = f"{fake_bin_dir}:{os.environ.get('PATH', '')}" + app_env = { + "PATH": launch_path, + "CMUX_AGENT_HOOK_STATE_DIR": str(hook_state_dir), + } + + def remove_hook_state() -> None: + for hook_state in hook_state_files.values(): + hook_state.unlink(missing_ok=True) + + _kill_existing(app_path) + snapshot.unlink(missing_ok=True) + previous_snapshot.unlink(missing_ok=True) + remove_hook_state() + + try: + # Phase 1: seed one workspace per session. + _launch(app_path, socket_path, env_overrides=app_env) + client = _connect(socket_path) + try: + workspace_ids = [client.current_workspace()] + while len(workspace_ids) < len(SESSION_SPECS): + workspace_ids.append(client.new_workspace()) + time.sleep(0.3) + + entries_by_launcher: dict[str, list[dict]] = {} + for index, (launcher, session_id, _) in enumerate(SESSION_SPECS): + client.select_workspace(workspace_ids[index]) + time.sleep(0.3) + surfaces = client.list_surfaces() + if not surfaces: + failures.append(f"setup: expected a surface in workspace {index}") + continue + entries_by_launcher.setdefault(launcher, []).append( + _hook_session_entry( + session_id=session_id, + workspace_id=workspace_ids[index], + surface_id=surfaces[0][1], + cwd=os.getcwd(), + launcher=launcher, + executable_path=fake_bin_dir / launcher, + ) + ) + for launcher, entries in entries_by_launcher.items(): + _write_hook_state(hook_state_files[launcher], entries) + client.select_workspace(0) + time.sleep(0.4) + finally: + client.close() + if failures: + return _report(failures) + _quit(bundle_id, socket_path) + + # Prove relaunches use the persisted snapshot, not live hook files. + remove_hook_state() + + # Phase 2: clean relaunch resumes everything. + _launch(app_path, socket_path, env_overrides=app_env) + client = _connect(socket_path) + try: + _assert_all_sessions_resumed(client, "clean relaunch #1", failures) + finally: + client.close() + _quit(bundle_id, socket_path) + + # Phase 3: second clean relaunch (re-saved snapshot) resumes everything. + _launch(app_path, socket_path, env_overrides=app_env) + client = _connect(socket_path) + try: + _assert_all_sessions_resumed(client, "clean relaunch #2", failures) + finally: + client.close() + + # Phase 4: force-kill after the autosave window, relaunch, resume. + time.sleep(12.0) # > SessionPersistencePolicy.autosaveInterval + _force_kill(app_path, socket_path) + _launch(app_path, socket_path, env_overrides=app_env) + client = _connect(socket_path) + try: + _assert_all_sessions_resumed(client, "relaunch after SIGKILL", failures) + finally: + client.close() + _quit(bundle_id, socket_path) + + # Phase 5: corrupt the primary snapshot; relaunch must recover from + # the -previous backup instead of silently starting fresh. + if not snapshot.exists(): + failures.append("corrupt-snapshot phase: expected a primary snapshot after quit") + return _report(failures) + snapshot.write_text('{"version": 9999, "windows": [truncated-mid-w', encoding="utf-8") + + _launch(app_path, socket_path, env_overrides=app_env) + client = _connect(socket_path) + try: + _assert_all_sessions_resumed(client, "relaunch with corrupt primary snapshot", failures) + finally: + client.close() + if not previous_snapshot.exists(): + failures.append( + "corrupt-snapshot phase: -previous backup snapshot was deleted; " + "restore-session recovery is impossible after a corrupt primary snapshot" + ) + _quit(bundle_id, socket_path) + finally: + _kill_existing(app_path) + socket_path.unlink(missing_ok=True) + snapshot.unlink(missing_ok=True) + previous_snapshot.unlink(missing_ok=True) + remove_hook_state() + + return _report(failures) + + +def _report(failures: list[str]) -> int: + if failures: + print("FAIL:") + for failure in failures: + print(f"- {failure}") + return 1 + print("PASS: agent sessions survive clean relaunch, repeat relaunch, SIGKILL, and corrupt-snapshot recovery") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 5543b2dd24e0f1693b7de997ccc02d1677ac5b6f Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:32:35 -0700 Subject: [PATCH 2/5] Recover session restore from the -previous backup when the primary snapshot is corrupt SessionPersistenceStore.load() treated a corrupt primary snapshot the same as a missing one: startup silently began a fresh session and syncManualRestoreSnapshotCache deleted session--previous.json, the only remaining copy of the user's workspaces, so restore-session could not recover anything either. loadOutcome now distinguishes a missing snapshot (clean state) from an unusable one (unreadable data, decode failure, schema drift, anomalous empty window list). When the primary is unusable, the backup is preserved and startup restore falls back to it, so workspaces and tracked agent sessions come back automatically. Also captures launch environment in the stress harness hook entries so fake-claude resume wins the PATH lookup (claude resume intentionally routes through the wrapper shim / PATH instead of the captured executable). --- Sources/SessionPersistence.swift | 50 +++++++++++++++---- ...test_session_restore_stress_kill_cycles.py | 6 +++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index adf5ffc1c946..7af6048f6cbe 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -1868,13 +1868,29 @@ struct AppSessionSnapshot: Codable, Sendable { } enum SessionPersistenceStore { + enum SnapshotLoadOutcome { + case loaded(AppSessionSnapshot) + /// No snapshot file on disk: a genuinely clean state. + case missing + /// A snapshot file exists but cannot be restored (unreadable data, + /// decode failure, schema version drift, or an anomalous empty + /// window list; empty states remove the file instead of writing it). + case unusable + } + + static func loadOutcome(fileURL: URL) -> SnapshotLoadOutcome { + guard FileManager.default.fileExists(atPath: fileURL.path) else { return .missing } + guard let data = try? Data(contentsOf: fileURL) else { return .unusable } + let decoder = JSONDecoder() + guard let snapshot = try? decoder.decode(AppSessionSnapshot.self, from: data) else { return .unusable } + guard snapshot.version == SessionSnapshotSchema.currentVersion else { return .unusable } + guard !snapshot.windows.isEmpty else { return .unusable } + return .loaded(snapshot) + } + static func load(fileURL: URL? = nil) -> AppSessionSnapshot? { guard let fileURL = fileURL ?? defaultSnapshotFileURL() else { return nil } - guard let data = try? Data(contentsOf: fileURL) else { return nil } - let decoder = JSONDecoder() - guard let snapshot = try? decoder.decode(AppSessionSnapshot.self, from: data) else { return nil } - guard snapshot.version == SessionSnapshotSchema.currentVersion else { return nil } - guard !snapshot.windows.isEmpty else { return nil } + guard case .loaded(let snapshot) = loadOutcome(fileURL: fileURL) else { return nil } return snapshot } @@ -1932,11 +1948,17 @@ enum SessionPersistenceStore { bundleIdentifier: bundleIdentifier, appSupportDirectory: appSupportDirectory ) else { return } - guard let snapshot = load(fileURL: primaryURL) else { + switch loadOutcome(fileURL: primaryURL) { + case .loaded(let snapshot): + _ = save(snapshot, fileURL: backupURL) + case .missing: removeSnapshot(fileURL: backupURL) - return + case .unusable: + // The primary snapshot exists but cannot be restored. Keep the + // backup: it is the only remaining recovery path for the user's + // sessions (startup fallback and `cmux restore-session`). + break } - _ = save(snapshot, fileURL: backupURL) } static func loadStartupSnapshot( @@ -1947,7 +1969,17 @@ enum SessionPersistenceStore { bundleIdentifier: bundleIdentifier, appSupportDirectory: appSupportDirectory ) else { return nil } - return load(fileURL: primaryURL) + switch loadOutcome(fileURL: primaryURL) { + case .loaded(let snapshot): + return snapshot + case .missing: + return nil + case .unusable: + return loadReopenSessionSnapshot( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: appSupportDirectory + ) + } } static func defaultSnapshotFileURL( diff --git a/tests/test_session_restore_stress_kill_cycles.py b/tests/test_session_restore_stress_kill_cycles.py index 716e5564eafe..f26648f5588a 100644 --- a/tests/test_session_restore_stress_kill_cycles.py +++ b/tests/test_session_restore_stress_kill_cycles.py @@ -205,7 +205,11 @@ def _hook_session_entry( cwd: str, launcher: str, executable_path: Path, + environment: dict[str, str], ) -> dict: + # The captured environment matters for claude: resume routes through the + # wrapper shim / bare `claude` on PATH instead of the captured executable, + # so the fake binary must win the PATH lookup in the restored shell. return { "sessionId": session_id, "workspaceId": workspace_id, @@ -216,6 +220,7 @@ def _hook_session_entry( "executablePath": str(executable_path), "arguments": [str(executable_path)], "workingDirectory": cwd, + "environment": environment, "capturedAt": time.time(), "source": "test", }, @@ -325,6 +330,7 @@ def remove_hook_state() -> None: cwd=os.getcwd(), launcher=launcher, executable_path=fake_bin_dir / launcher, + environment={"PATH": launch_path, "SHELL": "/bin/zsh"}, ) ) for launcher, entries in entries_by_launcher.items(): From c1db91f6958886821c4474b1afc6428163a9cfc4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:55:29 -0700 Subject: [PATCH 3/5] Repair claude coverage in relaunch/stress harnesses: transcript gating and wrapper resolution Claude hook records are only restorable when their transcript exists on disk (hookRecordIsRestorable), and claude resume routes through the cmux claude wrapper, which resolves the real binary instead of the captured executable. The relaunch harness silently lost its claude assertion when those behaviors landed (it is not run in CI): fake claude sessions were dropped at index load, and when they did resume the real claude binary ran instead of the fake. Both harnesses now write a transcriptPath for claude sessions, point CMUX_CUSTOM_CLAUDE_PATH at the fake binary, and match resume markers as order-agnostic tokens on one line (the wrapper inserts its own arguments around --resume). --- ...session_relaunch_resumes_agent_sessions.py | 65 ++++++++++++------- ...test_session_restore_stress_kill_cycles.py | 44 +++++++++---- 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/tests/test_session_relaunch_resumes_agent_sessions.py b/tests/test_session_relaunch_resumes_agent_sessions.py index 87f31baa8d4e..953d980c5ce3 100644 --- a/tests/test_session_relaunch_resumes_agent_sessions.py +++ b/tests/test_session_relaunch_resumes_agent_sessions.py @@ -155,29 +155,30 @@ def _write_hook_state( executable_path: Path, arguments: list[str] | None = None, environment: dict[str, str] | None = None, + transcript_path: Path | None = None, ) -> None: path.parent.mkdir(parents=True, exist_ok=True) - payload = { - "version": 1, - "sessions": { - session_id: { - "sessionId": session_id, - "workspaceId": workspace_id, - "surfaceId": surface_id, - "cwd": cwd, - "launchCommand": { - "launcher": launcher, - "executablePath": str(executable_path), - "arguments": arguments or [str(executable_path)], - "workingDirectory": cwd, - "environment": environment, - "capturedAt": time.time(), - "source": "test", - }, - "updatedAt": time.time(), - } + session: dict = { + "sessionId": session_id, + "workspaceId": workspace_id, + "surfaceId": surface_id, + "cwd": cwd, + "launchCommand": { + "launcher": launcher, + "executablePath": str(executable_path), + "arguments": arguments or [str(executable_path)], + "workingDirectory": cwd, + "environment": environment, + "capturedAt": time.time(), + "source": "test", }, + "updatedAt": time.time(), } + if transcript_path is not None: + # Claude hook records are only restorable when their transcript + # exists on disk (hookRecordIsRestorable). + session["transcriptPath"] = str(transcript_path) + payload = {"version": 1, "sessions": {session_id: session}} path.write_text(json.dumps(payload), encoding="utf-8") @@ -196,9 +197,12 @@ def main() -> int: snapshot = _snapshot_path(bundle_id) previous_snapshot = _snapshot_path(bundle_id, suffix="-previous") codex_expected = "CMUX_FAKE_CODEX_RESUME:resume codex-session-relaunch-2923" - claude_expected = ( - "CMUX_FAKE_CLAUDE_RESUME:--resume claude-session-relaunch-2923 " - "--dangerously-skip-permissions" + # The cmux claude wrapper inserts its own arguments around --resume, so + # claude expectations are order-agnostic tokens that must share one line. + claude_expected_tokens = ( + "CMUX_FAKE_CLAUDE_RESUME:", + "--resume claude-session-relaunch-2923", + "--dangerously-skip-permissions", ) opencode_expected = "CMUX_FAKE_OPENCODE_RESUME:--session opencode-session-relaunch-2923" pi_expected = "CMUX_FAKE_PI_RESUME:--session pi-session-relaunch-2923" @@ -220,6 +224,9 @@ def main() -> int: app_env = { "PATH": launch_path, "CMUX_AGENT_HOOK_STATE_DIR": str(hook_state_dir), + # Claude resume routes through the cmux claude wrapper, which + # resolves the real binary; point it at the fake one instead. + "CMUX_CUSTOM_CLAUDE_PATH": str(fake_bin_dir / "claude"), } _kill_existing(app_path) @@ -257,6 +264,8 @@ def main() -> int: if not claude_surfaces: failures.append("expected a Claude workspace surface during setup") else: + claude_transcript = Path(td) / "claude-transcript.jsonl" + claude_transcript.write_text('{"type":"user"}\n', encoding="utf-8") _write_hook_state( claude_hook_state, session_id="claude-session-relaunch-2923", @@ -275,6 +284,7 @@ def main() -> int: "SHELL": "/bin/zsh", "UNSAFE_TOKEN": "must-not-restore", }, + transcript_path=claude_transcript, ) opencode_workspace_id = client.new_workspace() @@ -347,6 +357,15 @@ def workspace_contains(index: int, expected: str) -> bool: client.select_workspace(index) return expected in _read_scrollback(client) + def workspace_line_contains(index: int, tokens: tuple[str, ...]) -> bool: + if len(client.list_workspaces()) <= index: + return False + client.select_workspace(index) + return any( + all(token in line for token in tokens) + for line in _read_scrollback(client).splitlines() + ) + if not _wait_for_condition(12.0, lambda: workspace_contains(0, codex_expected)): client.select_workspace(0) scrollback_tail = "\n".join(_read_scrollback(client).splitlines()[-20:]) @@ -355,7 +374,7 @@ def workspace_contains(index: int, expected: str) -> bool: f"tail:\n{scrollback_tail}" ) - if not _wait_for_condition(12.0, lambda: workspace_contains(1, claude_expected)): + if not _wait_for_condition(12.0, lambda: workspace_line_contains(1, claude_expected_tokens)): client.select_workspace(1) scrollback_tail = "\n".join(_read_scrollback(client).splitlines()[-20:]) failures.append( diff --git a/tests/test_session_restore_stress_kill_cycles.py b/tests/test_session_restore_stress_kill_cycles.py index f26648f5588a..5a113d17ac96 100644 --- a/tests/test_session_restore_stress_kill_cycles.py +++ b/tests/test_session_restore_stress_kill_cycles.py @@ -32,16 +32,25 @@ from cmux import cmux +# (launcher, session id, marker tokens). A session counts as resumed when one +# scrollback line contains every token: the fake-agent prefix proves the fake +# binary ran (the typed resume command alone does not contain it), and the +# session token proves which session it was. Claude tokens stay order-agnostic +# because the cmux claude wrapper inserts its own arguments around --resume. SESSION_SPECS = [ - ("claude", "claude-stress-0", "CMUX_FAKE_CLAUDE_RESUME:--resume claude-stress-0"), - ("claude", "claude-stress-1", "CMUX_FAKE_CLAUDE_RESUME:--resume claude-stress-1"), - ("codex", "codex-stress-0", "CMUX_FAKE_CODEX_RESUME:resume codex-stress-0"), - ("codex", "codex-stress-1", "CMUX_FAKE_CODEX_RESUME:resume codex-stress-1"), - ("opencode", "opencode-stress-0", "CMUX_FAKE_OPENCODE_RESUME:--session opencode-stress-0"), - ("opencode", "opencode-stress-1", "CMUX_FAKE_OPENCODE_RESUME:--session opencode-stress-1"), + ("claude", "claude-stress-0", ("CMUX_FAKE_CLAUDE_RESUME:", "--resume claude-stress-0")), + ("claude", "claude-stress-1", ("CMUX_FAKE_CLAUDE_RESUME:", "--resume claude-stress-1")), + ("codex", "codex-stress-0", ("CMUX_FAKE_CODEX_RESUME:", "resume codex-stress-0")), + ("codex", "codex-stress-1", ("CMUX_FAKE_CODEX_RESUME:", "resume codex-stress-1")), + ("opencode", "opencode-stress-0", ("CMUX_FAKE_OPENCODE_RESUME:", "--session opencode-stress-0")), + ("opencode", "opencode-stress-1", ("CMUX_FAKE_OPENCODE_RESUME:", "--session opencode-stress-1")), ] +def _marker_found(combined: str, tokens: tuple[str, ...]) -> bool: + return any(all(token in line for token in tokens) for line in combined.splitlines()) + + def _bundle_id(app_path: Path) -> str: info_path = app_path / "Contents" / "Info.plist" if not info_path.exists(): @@ -206,11 +215,11 @@ def _hook_session_entry( launcher: str, executable_path: Path, environment: dict[str, str], + transcript_path: Path | None = None, ) -> dict: - # The captured environment matters for claude: resume routes through the - # wrapper shim / bare `claude` on PATH instead of the captured executable, - # so the fake binary must win the PATH lookup in the restored shell. - return { + # Claude hook records are only restorable when their transcript exists on + # disk (hookRecordIsRestorable), so claude entries carry a transcriptPath. + entry = { "sessionId": session_id, "workspaceId": workspace_id, "surfaceId": surface_id, @@ -226,6 +235,9 @@ def _hook_session_entry( }, "updatedAt": time.time(), } + if transcript_path is not None: + entry["transcriptPath"] = str(transcript_path) + return entry def _collect_all_scrollbacks(client: cmux) -> str: @@ -249,12 +261,12 @@ def all_present() -> bool: if len(client.list_workspaces()) < len(SESSION_SPECS): return False combined = _collect_all_scrollbacks(client) - return all(marker in combined for marker in expected_markers) + return all(_marker_found(combined, marker) for marker in expected_markers) if _wait_for_condition(timeout, all_present): return combined = _collect_all_scrollbacks(client) - missing = [marker for marker in expected_markers if marker not in combined] + missing = [marker for marker in expected_markers if not _marker_found(combined, marker)] workspace_count = len(client.list_workspaces()) failures.append( f"{phase}: {len(missing)}/{len(expected_markers)} sessions did not resume " @@ -293,6 +305,9 @@ def main() -> int: app_env = { "PATH": launch_path, "CMUX_AGENT_HOOK_STATE_DIR": str(hook_state_dir), + # Claude resume routes through the cmux claude wrapper, which + # resolves the real binary; point it at the fake one instead. + "CMUX_CUSTOM_CLAUDE_PATH": str(fake_bin_dir / "claude"), } def remove_hook_state() -> None: @@ -322,6 +337,10 @@ def remove_hook_state() -> None: if not surfaces: failures.append(f"setup: expected a surface in workspace {index}") continue + transcript_path: Path | None = None + if launcher == "claude": + transcript_path = Path(td) / f"transcript-{session_id}.jsonl" + transcript_path.write_text('{"type":"user"}\n', encoding="utf-8") entries_by_launcher.setdefault(launcher, []).append( _hook_session_entry( session_id=session_id, @@ -331,6 +350,7 @@ def remove_hook_state() -> None: launcher=launcher, executable_path=fake_bin_dir / launcher, environment={"PATH": launch_path, "SHELL": "/bin/zsh"}, + transcript_path=transcript_path, ) ) for launcher, entries in entries_by_launcher.items(): From ebe0a3c514dcf5281c4343f6cb377d1b158a1c43 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:58:48 -0700 Subject: [PATCH 4/5] Compact snapshot backup tests behind a shared fixture; refresh swift file length budget --- .github/swift-file-length-budget.tsv | 10 +- cmuxTests/SessionPersistenceTests.swift | 150 +++++++++--------------- 2 files changed, 62 insertions(+), 98 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index ded99ec365c0..22bc7db22ab8 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -5,7 +5,7 @@ 22840 Sources/TerminalController.swift 19955 Sources/Workspace.swift 19248 Sources/ContentView.swift -18057 Sources/AppDelegate.swift +18021 Sources/AppDelegate.swift 16539 Sources/GhosttyTerminalView.swift 13608 Sources/Panels/BrowserPanel.swift 11916 cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -15,8 +15,8 @@ 7198 cmuxTests/WorkspaceUnitTests.swift 6948 cmuxTests/WorkspaceRemoteConnectionTests.swift 6542 cmuxTests/GhosttyConfigTests.swift +6329 cmuxTests/SessionPersistenceTests.swift 6299 cmuxTests/TerminalAndGhosttyTests.swift -6220 cmuxTests/SessionPersistenceTests.swift 6153 CLI/cmux_open.swift 6071 Sources/TextBoxInput.swift 5482 cmuxTests/BrowserConfigTests.swift @@ -44,7 +44,7 @@ 2290 Sources/FileExplorerView.swift 2260 Sources/TerminalWindowPortal.swift 2209 Sources/Mobile/MobileHostService.swift -2138 Sources/SessionPersistence.swift +2191 Sources/SessionPersistence.swift 2123 cmuxTests/ShortcutAndCommandPaletteTests.swift 2117 cmuxTests/CmuxConfigTests.swift 1996 Sources/KeyboardShortcutSettingsFileStore.swift @@ -104,6 +104,7 @@ 830 Sources/TaskManagerTypes.swift 810 Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift 787 Sources/ClosedItemHistory.swift +774 cmuxUITests/BrowserFixtureInteractionUITests.swift 768 Sources/MainWindowFocusController.swift 760 Packages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swift 757 Packages/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift @@ -134,7 +135,6 @@ 650 Sources/Panels/MarkdownRemoteImageLoader.swift 649 Sources/CmuxTopSnapshot.swift 640 cmuxTests/CommandPaletteNucleoFFITests.swift -638 Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift 630 Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutWhenClause.swift 627 Sources/WorkspaceRemoteConfiguration.swift 621 cmuxUITests/RightSidebarChromeHeightUITests.swift @@ -177,6 +177,7 @@ 519 Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/stress-two-column-cockpit-sidebar.swift 518 Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/stress-git-review-queue-command-deck.swift 516 Sources/CmuxConfigExecutor.swift +514 Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift 514 cmuxUITests/UpdatePillUITests.swift 509 Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerAdditionalPolicies.swift 507 Sources/TerminalControllerTopSupport.swift @@ -185,4 +186,3 @@ 504 cmuxTests/TerminalNotificationSocketActionTests.swift 502 Sources/CmuxEventPublishing.swift 502 Sources/Settings/ConfigSource.swift -774 cmuxUITests/BrowserFixtureInteractionUITests.swift diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index 2b1544f8700d..6ffe59caaff8 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -238,113 +238,91 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertEqual(loaded.windows.first?.sidebar.width, 321) } - func testSyncManualRestoreCachePreservesBackupWhenPrimarySnapshotIsCorrupt() throws { + private struct SnapshotBackupFixture { + let tempDir: URL + let bundleIdentifier: String + let primaryURL: URL + let backupURL: URL + + func writeCorruptPrimary() throws { + try FileManager.default.createDirectory( + at: primaryURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try Data("{\"version\": 9999, \"windows\": [truncated-mid-w".utf8).write(to: primaryURL) + } + } + + private func makeSnapshotBackupFixture(backupSnapshot: AppSessionSnapshot) throws -> SnapshotBackupFixture { let tempDir = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: tempDir) } - let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" - let primaryURL = try XCTUnwrap( - SessionPersistenceStore.defaultSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir - ) - ) - let backupURL = try XCTUnwrap( - SessionPersistenceStore.manualRestoreSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir + let fixture = SnapshotBackupFixture( + tempDir: tempDir, + bundleIdentifier: bundleIdentifier, + primaryURL: try XCTUnwrap( + SessionPersistenceStore.defaultSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) + ), + backupURL: try XCTUnwrap( + SessionPersistenceStore.manualRestoreSnapshotFileURL( + bundleIdentifier: bundleIdentifier, + appSupportDirectory: tempDir + ) ) ) + XCTAssertTrue(SessionPersistenceStore.save(backupSnapshot, fileURL: fixture.backupURL)) + return fixture + } - XCTAssertTrue( - SessionPersistenceStore.save( - makeSnapshot(version: SessionSnapshotSchema.currentVersion), - fileURL: backupURL - ) - ) - try FileManager.default.createDirectory( - at: primaryURL.deletingLastPathComponent(), - withIntermediateDirectories: true + func testSyncManualRestoreCachePreservesBackupWhenPrimarySnapshotIsCorrupt() throws { + let fixture = try makeSnapshotBackupFixture( + backupSnapshot: makeSnapshot(version: SessionSnapshotSchema.currentVersion) ) - try Data("{\"version\": 9999, \"windows\": [truncated-mid-w".utf8).write(to: primaryURL) + defer { try? FileManager.default.removeItem(at: fixture.tempDir) } + try fixture.writeCorruptPrimary() SessionPersistenceStore.syncManualRestoreSnapshotCache( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir + bundleIdentifier: fixture.bundleIdentifier, + appSupportDirectory: fixture.tempDir ) XCTAssertNotNil( - SessionPersistenceStore.load(fileURL: backupURL), + SessionPersistenceStore.load(fileURL: fixture.backupURL), "A corrupt primary snapshot must not destroy the restore-session backup" ) } func testSyncManualRestoreCacheRemovesBackupWhenPrimarySnapshotIsMissing() throws { - let tempDir = FileManager.default.temporaryDirectory - .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: tempDir) } - - let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" - let backupURL = try XCTUnwrap( - SessionPersistenceStore.manualRestoreSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir - ) - ) - - XCTAssertTrue( - SessionPersistenceStore.save( - makeSnapshot(version: SessionSnapshotSchema.currentVersion), - fileURL: backupURL - ) + let fixture = try makeSnapshotBackupFixture( + backupSnapshot: makeSnapshot(version: SessionSnapshotSchema.currentVersion) ) + defer { try? FileManager.default.removeItem(at: fixture.tempDir) } SessionPersistenceStore.syncManualRestoreSnapshotCache( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir + bundleIdentifier: fixture.bundleIdentifier, + appSupportDirectory: fixture.tempDir ) XCTAssertNil( - SessionPersistenceStore.load(fileURL: backupURL), + SessionPersistenceStore.load(fileURL: fixture.backupURL), "A genuinely absent primary snapshot still clears the stale backup" ) } func testStartupSnapshotLoadRecoversFromBackupWhenPrimarySnapshotIsCorrupt() throws { - let tempDir = FileManager.default.temporaryDirectory - .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: tempDir) } - - let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" - let primaryURL = try XCTUnwrap( - SessionPersistenceStore.defaultSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir - ) - ) - let backupURL = try XCTUnwrap( - SessionPersistenceStore.manualRestoreSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir - ) - ) - var backupSnapshot = makeSnapshot(version: SessionSnapshotSchema.currentVersion) backupSnapshot.windows[0].sidebar.width = 321 - XCTAssertTrue(SessionPersistenceStore.save(backupSnapshot, fileURL: backupURL)) - try FileManager.default.createDirectory( - at: primaryURL.deletingLastPathComponent(), - withIntermediateDirectories: true - ) - try Data("not a session snapshot".utf8).write(to: primaryURL) + let fixture = try makeSnapshotBackupFixture(backupSnapshot: backupSnapshot) + defer { try? FileManager.default.removeItem(at: fixture.tempDir) } + try fixture.writeCorruptPrimary() let loaded = SessionPersistenceStore.loadStartupSnapshot( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir + bundleIdentifier: fixture.bundleIdentifier, + appSupportDirectory: fixture.tempDir ) XCTAssertEqual( @@ -355,29 +333,15 @@ final class SessionPersistenceTests: XCTestCase { } func testStartupSnapshotLoadReturnsNilWhenPrimarySnapshotIsMissing() throws { - let tempDir = FileManager.default.temporaryDirectory - .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - defer { try? FileManager.default.removeItem(at: tempDir) } - - let bundleIdentifier = "dev.cmux.tests.\(UUID().uuidString)" - let backupURL = try XCTUnwrap( - SessionPersistenceStore.manualRestoreSnapshotFileURL( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir - ) - ) - XCTAssertTrue( - SessionPersistenceStore.save( - makeSnapshot(version: SessionSnapshotSchema.currentVersion), - fileURL: backupURL - ) + let fixture = try makeSnapshotBackupFixture( + backupSnapshot: makeSnapshot(version: SessionSnapshotSchema.currentVersion) ) + defer { try? FileManager.default.removeItem(at: fixture.tempDir) } XCTAssertNil( SessionPersistenceStore.loadStartupSnapshot( - bundleIdentifier: bundleIdentifier, - appSupportDirectory: tempDir + bundleIdentifier: fixture.bundleIdentifier, + appSupportDirectory: fixture.tempDir ), "A clean start without a primary snapshot must not resurrect the backup" ) From aa06b1370b4307d3c506a9ddd56a553f9bec0509 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Thu, 11 Jun 2026 16:13:18 -0700 Subject: [PATCH 5/5] Address review: log corrupt-primary backup fallback, exercise restore-session in corrupt phase Adds a session.restore.primaryUnusable debug-log line when startup falls back from an unusable primary snapshot, and extends the corrupt-snapshot stress phase to run the bundled cmux restore-session verb, asserting it reopens the backed-up session in a new window (window count, since the restored workspaces share ids with the startup fallback restore). --- Sources/SessionPersistence.swift | 9 +++- ...test_session_restore_stress_kill_cycles.py | 54 +++++++++++++++++-- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index 7af6048f6cbe..3717255e54b9 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -1975,10 +1975,17 @@ enum SessionPersistenceStore { case .missing: return nil case .unusable: - return loadReopenSessionSnapshot( + let backup = loadReopenSessionSnapshot( bundleIdentifier: bundleIdentifier, appSupportDirectory: appSupportDirectory ) +#if DEBUG + cmuxDebugLog( + "session.restore.primaryUnusable path=\(primaryURL.path) " + + "backupRecovered=\(backup != nil ? 1 : 0)" + ) +#endif + return backup } } diff --git a/tests/test_session_restore_stress_kill_cycles.py b/tests/test_session_restore_stress_kill_cycles.py index 5a113d17ac96..4ff9be56a8a9 100644 --- a/tests/test_session_restore_stress_kill_cycles.py +++ b/tests/test_session_restore_stress_kill_cycles.py @@ -405,13 +405,57 @@ def remove_hook_state() -> None: client = _connect(socket_path) try: _assert_all_sessions_resumed(client, "relaunch with corrupt primary snapshot", failures) + + if not previous_snapshot.exists(): + failures.append( + "corrupt-snapshot phase: -previous backup snapshot was deleted; " + "restore-session recovery is impossible after a corrupt primary snapshot" + ) + else: + # The manual `cmux restore-session` recovery entrypoint must + # also still work from the preserved backup. It reopens the + # backed-up workspaces in a new window (with the same + # workspace ids as the startup fallback restore, since both + # read the same backup), so assert on the window count. + cli_path = app_path / "Contents" / "Resources" / "bin" / "cmux" + restore_env = dict(os.environ) + restore_env["CMUX_SOCKET_PATH"] = str(socket_path) + + def window_count() -> int: + result = subprocess.run( + [str(cli_path), "list-windows", "--json"], + capture_output=True, + text=True, + env=restore_env, + ) + try: + return len(json.loads(result.stdout)) + except (json.JSONDecodeError, TypeError): + return -1 + + windows_before = window_count() + restore_proc = subprocess.run( + [str(cli_path), "restore-session"], + capture_output=True, + text=True, + env=restore_env, + ) + if restore_proc.returncode != 0 or restore_proc.stdout.strip() != "OK": + failures.append( + "corrupt-snapshot phase: restore-session failed after backup-preserving " + f"relaunch; rc={restore_proc.returncode} stdout={restore_proc.stdout!r} " + f"stderr={restore_proc.stderr!r}" + ) + elif windows_before < 1 or not _wait_for_condition( + 20.0, lambda: window_count() > windows_before + ): + failures.append( + "corrupt-snapshot phase: restore-session did not reopen the backed-up " + f"session in a new window (windows before={windows_before}, " + f"after={window_count()})" + ) finally: client.close() - if not previous_snapshot.exists(): - failures.append( - "corrupt-snapshot phase: -previous backup snapshot was deleted; " - "restore-session recovery is impossible after a corrupt primary snapshot" - ) _quit(bundle_id, socket_path) finally: _kill_existing(app_path)