Fix session autosave disk and memory growth#2886
Open
austinywang wants to merge 11 commits intomainfrom
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughReplaces fingerprint-based autosave skipping with explicit dirty-state tracking and debounced scheduling, adds window geometry/lifecycle observers, wires many state properties to mark session-dirty on semantic changes, increases autosave interval to 30s, and adapts tests to schedule-based behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Components
participant Delegate as AppDelegate
participant Scheduler as Scheduler
participant Timer as Timer Tick
participant Persist as SessionPersistenceStore
rect rgba(100, 150, 255, 0.5)
Note over UI,Delegate: Dirty-state request flow
UI->>Delegate: requestSessionSnapshotDirty(reason)
Delegate->>Delegate: sessionSnapshotIsDirty = true
Delegate->>Scheduler: scheduleSessionAutosave(after, source, allowDelayExtension)
Scheduler->>Scheduler: compute deadline & check shouldKeepExistingSchedule
Scheduler->>Timer: reschedule timer (dormant/distantFuture semantics)
end
rect rgba(150, 200, 100, 0.5)
Note over Timer,Persist: Autosave execution
Timer->>Delegate: runSessionAutosaveTick(source)
alt snapshot clean
Delegate->>Delegate: skip save
else snapshot dirty
Delegate->>Persist: saveSessionSnapshot(...)
Persist->>Persist: encode & atomic write (autoreleasepool)
Persist-->>Delegate: completion
Delegate->>Delegate: completeSessionSnapshotPersistence(...)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 2
🧹 Nitpick comments (1)
Sources/AppDelegate.swift (1)
2221-2230: Please verify theseMainActor.assumeIsolatedfast paths are actor-safe.
Thread.isMainThreadandNotificationCenter(..., queue: .main)do not, by themselves, prove main-actor isolation. If either path runs on the main thread outside the main-actor executor,assumeIsolatedis fragile here. Hopping explicitly withTask {@mainactor... }is the safer pattern.Possible fix
nonisolated static func requestSessionSnapshotDirty(reason: String) { - if Thread.isMainThread { - MainActor.assumeIsolated { - AppDelegate.shared?.markSessionSnapshotDirty(reason: reason) - } - return - } - Task { `@MainActor` in AppDelegate.shared?.markSessionSnapshotDirty(reason: reason) } }let observe: (Notification.Name, String) -> Void = { [weak self, weak window] name, reason in guard let self else { return } let observer = center.addObserver( forName: name, object: window, queue: .main ) { [weak self] _ in - guard let self else { return } - MainActor.assumeIsolated { - self.markSessionSnapshotDirty(reason: reason) - } + Task { `@MainActor` [weak self] in + self?.markSessionSnapshotDirty(reason: reason) + } } context.sessionSnapshotWindowObservers.append(observer) }Also applies to: 4770-4780
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 2221 - 2230, The current nonisolated static requestSessionSnapshotDirty(reason:) uses Thread.isMainThread with MainActor.assumeIsolated which is unsafe; replace the assumeIsolated fast-path with an explicit hop to the main actor (use Task { `@MainActor` in AppDelegate.shared?.markSessionSnapshotDirty(reason: reason) }) so both the main-thread and non-main-thread paths always execute on the MainActor, and apply the same change to the other occurrences (the similar block around lines 4770-4780 and any NotificationCenter callbacks that use .main queue or assumeIsolated) to ensure actor-safe main-actor execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 4519-4520: runSessionAutosaveTick/saveSessionSnapshot currently
clear sessionSnapshotIsDirty and sessionAutosaveScheduledDeadlineUptime
immediately after enqueueing work, which allows overlapping writes because
actual disk I/O runs later on sessionPersistenceQueue; instead, change the flow
so those flags are only cleared on the main actor after the persistence queue
finishes (add a completion callback from the persistence write executed back on
`@MainActor`), keep the write block (the actual
SessionPersistenceStore.save/remove and persistedWindowGeometry writes)
executing on sessionPersistenceQueue, and if sessionSnapshotIsDirty was set
while the write was in flight, schedule an immediate retry; update calls at
runSessionAutosaveTick, saveSessionSnapshot and any places clearing
sessionSnapshotIsDirty (lines near 4540-4541 and 4577-4586) to use this
completion-based clearing via persistSessionSnapshot.
In `@Sources/Workspace.swift`:
- Around line 6626-6630: The statusEntries didSet should not call
markSessionSnapshotDirty for transient/remote-only changes—since
restoreSessionSnapshot(_:) drops statusEntries, stop dirtying the session from
any statusEntries updates (remove the markSessionSnapshotDirty call in the
statusEntries didSet). For surfaceListeningPorts (the other block referenced
around lines 6659-6663), only call markSessionSnapshotDirty when the persisted
subset actually changes: compute a persistable/filtering view that strips
remote-detected ports (the same subset sessionPanelSnapshot(...) omits) and
compare that filtered value to the old filtered value before calling
markSessionSnapshotDirty; reference statusEntries, surfaceListeningPorts,
restoreSessionSnapshot(_:), and sessionPanelSnapshot(...) to locate the relevant
code and
SessionPersistenceTests.testSessionSnapshotSkipsTransientRemoteListeningPorts
for expected behavior.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 2221-2230: The current nonisolated static
requestSessionSnapshotDirty(reason:) uses Thread.isMainThread with
MainActor.assumeIsolated which is unsafe; replace the assumeIsolated fast-path
with an explicit hop to the main actor (use Task { `@MainActor` in
AppDelegate.shared?.markSessionSnapshotDirty(reason: reason) }) so both the
main-thread and non-main-thread paths always execute on the MainActor, and apply
the same change to the other occurrences (the similar block around lines
4770-4780 and any NotificationCenter callbacks that use .main queue or
assumeIsolated) to ensure actor-safe main-actor execution.
🪄 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: c89dc844-44af-4841-abd4-99f6d840d417
📒 Files selected for processing (8)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/BrowserPanel.swiftSources/SessionPersistence.swiftSources/SidebarSelectionState.swiftSources/TabManager.swiftSources/Workspace.swift
Contributor
Greptile SummaryThis PR replaces the periodic-timer + hash-fingerprint autosave approach with an event-driven dirty-flag approach: any persisted-state change marks the session dirty and schedules a one-shot 30 s debounce timer (using Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant State as State Change<br/>(Workspace/Sidebar/Browser/Window)
participant WD as markSessionSnapshotDirty()
participant SD as scheduleSessionAutosave()<br/>allowDelayExtension:false
participant Timer as DispatchSourceTimer<br/>(one-shot, .distantFuture)
participant Tick as runSessionAutosaveTick()
participant Save as saveSessionSnapshot()
participant Queue as sessionPersistenceQueue<br/>(async disk write)
State->>WD: dirty event (e.g. currentDirectory, url, tabs)
WD->>WD: "sessionSnapshotIsDirty = true"
WD->>SD: after: 30s
SD->>SD: "existingDeadline set? (no → schedule)<br/>(yes → now+30s ≥ existing → SKIP)"
SD->>Timer: reschedule to now+30s
Note over Timer: Subsequent dirty events<br/>→ SKIP (deadline preserved)
Timer-->>Tick: fires after 30 s
Tick->>Tick: "sessionAutosaveScheduledDeadlineUptime = nil"
alt typing quiet period active
Tick->>SD: scheduleSessionAutosave(after: remainingQuietPeriod)
SD->>Timer: reschedule (retry)
else clean to save
Tick->>Save: saveSessionSnapshot()
Save->>Queue: persistSessionSnapshot (async)
Save->>WD: "sessionSnapshotIsDirty = false"
Save->>SD: "sessionAutosaveScheduledDeadlineUptime = nil"
end
Reviews (1): Last reviewed commit: "Fix autosave debounce regressions" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
4515-4543:⚠️ Potential issue | 🟠 MajorDirty/in-flight state is still cleared before the async write finishes.
saveSessionSnapshotclearssessionSnapshotIsDirty/sessionAutosaveScheduledDeadlineUptimeimmediately after enqueueing ontosessionPersistenceQueue, whilerunSessionAutosaveTickonly guards the enqueue/build phase. A slow disk write can still accumulate later autosaves behind it, which is the same backlog the PR is trying to eliminate.Also applies to: 4586-4596, 4656-4683
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 4515 - 4543, saveSessionSnapshot clears sessionSnapshotIsDirty and sessionAutosaveScheduledDeadlineUptime immediately after calling persistSessionSnapshot, which can enqueue an async write on sessionPersistenceQueue and allow later autosaves to pile up; change the logic so those dirty/in-flight flags are only cleared once the persistence finishes (i.e., move/reset sessionSnapshotIsDirty and sessionAutosaveScheduledDeadlineUptime into the completion path of persistSessionSnapshot or make persistSessionSnapshot return a completion/future and clear flags after it completes), keeping the existing immediate-clear behavior only for the synchronous branch (when writeSynchronously is true); update the same pattern for the other occurrences referenced (around lines with persistSessionSnapshot calls noted in the comment).
🧹 Nitpick comments (1)
cmuxTests/SessionPersistenceTests.swift (1)
517-525: Consider adding an equality boundary case for deadline comparison.Coverage currently checks
target > existingandtarget < existing; addingtarget == existingwould guard against future off-by-one/regression in schedule-keep logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/SessionPersistenceTests.swift` around lines 517 - 525, Add an equality-boundary test for the deadline comparison: create a test (e.g., testSessionAutosaveScheduleKeepsWhenDeadlinesEqual) that calls AppDelegate.shouldKeepExistingSessionAutosaveSchedule with existingDeadlineUptime and targetDeadlineUptime set to the same value (e.g., 200) and assert the expected behavior (keep the schedule) for the relevant allowDelayExtension cases; reference the existing test patterns in SessionPersistenceTests.swift and the target method shouldKeepExistingSessionAutosaveSchedule to mirror style and ensure future off-by-one regressions are caught.
🤖 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/AppDelegate.swift`:
- Around line 4515-4543: saveSessionSnapshot clears sessionSnapshotIsDirty and
sessionAutosaveScheduledDeadlineUptime immediately after calling
persistSessionSnapshot, which can enqueue an async write on
sessionPersistenceQueue and allow later autosaves to pile up; change the logic
so those dirty/in-flight flags are only cleared once the persistence finishes
(i.e., move/reset sessionSnapshotIsDirty and
sessionAutosaveScheduledDeadlineUptime into the completion path of
persistSessionSnapshot or make persistSessionSnapshot return a completion/future
and clear flags after it completes), keeping the existing immediate-clear
behavior only for the synchronous branch (when writeSynchronously is true);
update the same pattern for the other occurrences referenced (around lines with
persistSessionSnapshot calls noted in the comment).
---
Nitpick comments:
In `@cmuxTests/SessionPersistenceTests.swift`:
- Around line 517-525: Add an equality-boundary test for the deadline
comparison: create a test (e.g.,
testSessionAutosaveScheduleKeepsWhenDeadlinesEqual) that calls
AppDelegate.shouldKeepExistingSessionAutosaveSchedule with
existingDeadlineUptime and targetDeadlineUptime set to the same value (e.g.,
200) and assert the expected behavior (keep the schedule) for the relevant
allowDelayExtension cases; reference the existing test patterns in
SessionPersistenceTests.swift and the target method
shouldKeepExistingSessionAutosaveSchedule to mirror style and ensure future
off-by-one regressions are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 454eef55-b222-4d79-88ed-135a427c352e
📒 Files selected for processing (2)
Sources/AppDelegate.swiftcmuxTests/SessionPersistenceTests.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 70a9ef0. Configure here.
…isk-memory # Conflicts: # Sources/TabManager.swift # Sources/Workspace.swift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
4516-4544:⚠️ Potential issue | 🟠 MajorKeep autosave marked in-flight until
sessionPersistenceQueueactually finishes.
persistSessionSnapshotstill only queues the write for the async path, but Lines 4522-4524 and 4543-4544 clear the dirty/deadline state before that queued work runs. OncesaveSessionSnapshotreturns, the guard at Lines 4595-4597 no longer protects you, so a slowSessionPersistenceStore.save/removeSnapshotcan still build a backlog of writes behindsessionPersistenceQueueand reintroduce the disk/memory growth this PR is trying to eliminate. Please move the state clearing behind a main-actor completion frompersistSessionSnapshot, and immediately reschedule if anything marked the snapshot dirty during the queued write.Also applies to: 4587-4598
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 4516 - 4544, The code clears sessionSnapshotIsDirty and sessionAutosaveScheduledDeadlineUptime immediately after calling persistSessionSnapshot, but persistSessionSnapshot only queues the async write on sessionPersistenceQueue; move the clearing of sessionSnapshotIsDirty and sessionAutosaveScheduledDeadlineUptime into a completion handler (main-actor completion) invoked by persistSessionSnapshot once the queued save/remove has finished (or expose a completion from persistSessionSnapshot/saveSessionSnapshot/SessionPersistenceStore.save/removeSnapshot), and if sessionSnapshotIsDirty was set again while the queued write ran, immediately reschedule autosave (so you don't lose the dirty state); update both code paths (the nil snapshot early-return and the normal persist path around persistSessionSnapshot and saveSessionSnapshot) to use this completion-based clearing/reschedule logic.
🤖 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/AppDelegate.swift`:
- Around line 4516-4544: The code clears sessionSnapshotIsDirty and
sessionAutosaveScheduledDeadlineUptime immediately after calling
persistSessionSnapshot, but persistSessionSnapshot only queues the async write
on sessionPersistenceQueue; move the clearing of sessionSnapshotIsDirty and
sessionAutosaveScheduledDeadlineUptime into a completion handler (main-actor
completion) invoked by persistSessionSnapshot once the queued save/remove has
finished (or expose a completion from
persistSessionSnapshot/saveSessionSnapshot/SessionPersistenceStore.save/removeSnapshot),
and if sessionSnapshotIsDirty was set again while the queued write ran,
immediately reschedule autosave (so you don't lose the dirty state); update both
code paths (the nil snapshot early-return and the normal persist path around
persistSessionSnapshot and saveSessionSnapshot) to use this completion-based
clearing/reschedule logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35f09bfd-78ea-41b5-9dc4-cb551bf330f8
📒 Files selected for processing (5)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/Workspace.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/ContentView.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/TabManager.swift
- Sources/Workspace.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AppDelegate.swift (1)
4881-4908:⚠️ Potential issue | 🟠 MajorReinstall the close observer when a context is rebound to a new window.
Only the
createdbranch attachesNSWindow.willCloseNotification. If this path reuses an existingAppDelegate.MainWindowContextfor a differentNSWindow, the new window never unregisters, while the old window can still unregister the reused context via the laterwindowIdfallback. That leaves a live main window without a context and breaks follow-on session/save routing.Possible fix
private final class MainWindowContext { let windowId: UUID let tabManager: TabManager let sidebarState: SidebarState let sidebarSelectionState: SidebarSelectionState weak var window: NSWindow? var sessionSnapshotWindowObservers: [NSObjectProtocol] = [] + var windowCloseObserver: NSObjectProtocol? init( windowId: UUID, tabManager: TabManager, sidebarState: SidebarState, @@ deinit { sessionSnapshotWindowObservers.forEach(NotificationCenter.default.removeObserver) + if let windowCloseObserver { + NotificationCenter.default.removeObserver(windowCloseObserver) + } } }+ private func installMainWindowCloseObserver(for context: MainWindowContext, window: NSWindow) { + if let windowCloseObserver = context.windowCloseObserver { + NotificationCenter.default.removeObserver(windowCloseObserver) + } + context.windowCloseObserver = NotificationCenter.default.addObserver( + forName: NSWindow.willCloseNotification, + object: window, + queue: .main + ) { [weak self] note in + guard let self, let closing = note.object as? NSWindow else { return } + self.unregisterMainWindow(closing) + } + }- NotificationCenter.default.addObserver( - forName: NSWindow.willCloseNotification, - object: window, - queue: .main - ) { [weak self] note in - guard let self, let closing = note.object as? NSWindow else { return } - self.unregisterMainWindow(closing) - } } installSessionSnapshotWindowObservers(for: context, window: window) + installMainWindowCloseObserver(for: context, window: window)Based on learnings: during XCTest launch stabilization, a fallback main window can later be replaced by the real
WindowGroupwindow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 4881 - 4908, The close-notification observer is only added in the created branch, so when an existing MainWindowContext is rebound to a new NSWindow (in the mainWindowContexts[key] or windowId branches) the new window never gets an NSWindow.willCloseNotification observer and the old window may later unregister the reused context; fix by registering the same close observer in both places where you set existing.window = window (both the key-hit branch and the windowId fallback branch), using the same pattern as the created branch (NotificationCenter.default.addObserver with [weak self], guard to cast note.object to NSWindow, and calling unregisterMainWindow(closing)); ensure you bind the observer to the current window object and keep installSessionSnapshotWindowObservers(for: context, window: window) after registration so session/save routing remains correct.
🧹 Nitpick comments (1)
cmuxTests/SessionPersistenceTests.swift (1)
527-533: Consider adding the equality case whenallowDelayExtensionistrue.You already pin equality for
allowDelayExtension: false; adding the symmetrictruecase would lock idempotent behavior more explicitly.Suggested test addition
+ func testSessionAutosaveScheduleKeepsWhenDeadlinesAreEqualWithDelayExtensionEnabled() { + XCTAssertTrue( + AppDelegate.shouldKeepExistingSessionAutosaveSchedule( + allowDelayExtension: true, + existingDeadlineUptime: 200, + targetDeadlineUptime: 200 + ) + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/SessionPersistenceTests.swift` around lines 527 - 533, Add a symmetric assertion that verifies idempotent behavior when allowDelayExtension is true: in the testSessionAutosaveScheduleKeepsWhenDeadlinesAreEqual test add a call to AppDelegate.shouldKeepExistingSessionAutosaveSchedule with allowDelayExtension: true and both existingDeadlineUptime and targetDeadlineUptime set to the same value (e.g., 200) and assert true, so the equality case is covered for both allowDelayExtension branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 4699-4710: The synchronous quit path currently calls writeBlock()
directly and bypasses the serial sessionPersistenceQueue, allowing concurrent
async persistence tasks to later overwrite or delete the final snapshot; change
the synchronous branch so it dispatches writeBlock() synchronously onto
sessionPersistenceQueue (e.g., using sessionPersistenceQueue.sync) and only then
invoke completion (ensuring completion is called on the MainActor if the
existing Task { `@MainActor` in completion() } pattern is desired) so the final
save is serialized with other persistence operations—apply this change to the
code paths where synchronously is checked (used during
applicationWillTerminate(_:)) referencing writeBlock, sessionPersistenceQueue,
synchronously, and completion.
---
Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 4881-4908: The close-notification observer is only added in the
created branch, so when an existing MainWindowContext is rebound to a new
NSWindow (in the mainWindowContexts[key] or windowId branches) the new window
never gets an NSWindow.willCloseNotification observer and the old window may
later unregister the reused context; fix by registering the same close observer
in both places where you set existing.window = window (both the key-hit branch
and the windowId fallback branch), using the same pattern as the created branch
(NotificationCenter.default.addObserver with [weak self], guard to cast
note.object to NSWindow, and calling unregisterMainWindow(closing)); ensure you
bind the observer to the current window object and keep
installSessionSnapshotWindowObservers(for: context, window: window) after
registration so session/save routing remains correct.
---
Nitpick comments:
In `@cmuxTests/SessionPersistenceTests.swift`:
- Around line 527-533: Add a symmetric assertion that verifies idempotent
behavior when allowDelayExtension is true: in the
testSessionAutosaveScheduleKeepsWhenDeadlinesAreEqual test add a call to
AppDelegate.shouldKeepExistingSessionAutosaveSchedule with allowDelayExtension:
true and both existingDeadlineUptime and targetDeadlineUptime set to the same
value (e.g., 200) and assert true, so the equality case is covered for both
allowDelayExtension branches.
🪄 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: 2d57f399-6063-4dba-b78b-fe0884792322
📒 Files selected for processing (2)
Sources/AppDelegate.swiftcmuxTests/SessionPersistenceTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Notes
Note
Medium Risk
Touches core session autosave/persistence scheduling and adds many new dirty triggers, which could affect write frequency and session restore correctness if any edge cases are missed.
Overview
Session persistence is reworked to be dirty-driven and debounced: autosave now schedules a single timer at the earliest pending deadline (default interval raised to
30s), retries when typing is recent or async writes are in-flight, and tracks dirty state/generations to avoid save storms.Dirty tracking is broadened and made more precise across UI state changes: window move/resize/key/screen changes, sidebar visibility/width/selection, tab/workspace mutations (selection, panels, splits, focus, tab order), and browser persistence-relevant changes (URL/history availability, profile, zoom, devtools), with a lightweight history signature to detect same-URL navigations.
Snapshot persistence adds async completion handling and uses
autoreleasepoolaround JSON encode/decode, plus updates tests to cover the new scheduling policy and Bonsplit same-pane reorder dirty signaling.Reviewed by Cursor Bugbot for commit f7a68f3. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
New Features
Performance
Tests