From 9d1cb3623add6ef0938ecb7b0131e60dec6a4ee8 Mon Sep 17 00:00:00 2001 From: Morten Trydal Date: Wed, 18 Mar 2026 21:53:03 +0100 Subject: [PATCH] Debounce workspace persistence writes --- .../Persistence/WorkspacePersistence.swift | 65 +++++++++++++- .../WorkspaceManager+WorkspaceLifecycle.swift | 1 + .../Workspaces/WorkspaceManager.swift | 6 +- .../CoalescingWorkspacePersistenceTests.swift | 88 +++++++++++++++++++ .../WorkspaceManagerLifecycleTests.swift | 15 ++++ .../WorkspacePersistenceTests.swift | 27 ++++++ .../WorkspaceTestSupport.swift | 3 + 7 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 Tests/ShellraiserTests/CoalescingWorkspacePersistenceTests.swift diff --git a/Sources/Shellraiser/Services/Persistence/WorkspacePersistence.swift b/Sources/Shellraiser/Services/Persistence/WorkspacePersistence.swift index 37c3e82..571f031 100644 --- a/Sources/Shellraiser/Services/Persistence/WorkspacePersistence.swift +++ b/Sources/Shellraiser/Services/Persistence/WorkspacePersistence.swift @@ -7,6 +7,9 @@ protocol WorkspacePersisting { /// Persists workspaces to storage. func save(_ workspaces: [WorkspaceModel]) + + /// Forces any pending persistence work to complete immediately. + func flush() } /// Filesystem-backed persistence for workspace layout state. @@ -144,7 +147,6 @@ final class WorkspacePersistence: WorkspacePersisting { ) let encoder = JSONEncoder() - encoder.outputFormatting = [.prettyPrinted, .sortedKeys] encoder.dateEncodingStrategy = .iso8601 let data = try encoder.encode(workspaces) @@ -155,4 +157,65 @@ final class WorkspacePersistence: WorkspacePersisting { } } } + + /// Filesystem-backed writes are synchronous, so flushing is a no-op. + func flush() {} +} + +/// Debounces repeated workspace saves and persists only the latest snapshot. +final class CoalescingWorkspacePersistence: WorkspacePersisting { + private let backing: any WorkspacePersisting + private let debounceInterval: TimeInterval + private let coordinationQueue = DispatchQueue(label: "com.shellraiser.workspace-persistence") + private var pendingWorkspaces: [WorkspaceModel]? + private var saveWorkItem: DispatchWorkItem? + + /// Creates a coalescing wrapper around another persistence implementation. + init(backing: any WorkspacePersisting, debounceInterval: TimeInterval = 0.5) { + self.backing = backing + self.debounceInterval = max(0, debounceInterval) + } + + /// Returns the latest in-memory snapshot when a debounced save is pending. + func load() -> [WorkspaceModel]? { + coordinationQueue.sync { + pendingWorkspaces ?? backing.load() + } + } + + /// Stores the latest snapshot and resets the debounce timer. + func save(_ workspaces: [WorkspaceModel]) { + coordinationQueue.sync { + pendingWorkspaces = workspaces + saveWorkItem?.cancel() + + guard debounceInterval > 0 else { + persistPendingWorkspaces() + return + } + + let workItem = DispatchWorkItem { [weak self] in + self?.persistPendingWorkspaces() + } + saveWorkItem = workItem + coordinationQueue.asyncAfter(deadline: .now() + debounceInterval, execute: workItem) + } + } + + /// Cancels any scheduled save and writes the latest snapshot immediately. + func flush() { + coordinationQueue.sync { + saveWorkItem?.cancel() + persistPendingWorkspaces() + } + } + + /// Writes and clears the currently pending snapshot, if any. + private func persistPendingWorkspaces() { + guard let pendingWorkspaces else { return } + self.pendingWorkspaces = nil + saveWorkItem = nil + backing.save(pendingWorkspaces) + backing.flush() + } } diff --git a/Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift b/Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift index 066b87e..cc124b3 100644 --- a/Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift +++ b/Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift @@ -141,6 +141,7 @@ extension WorkspaceManager { /// Persists the current workspace collection. func save() { persistence.save(workspaces) + persistence.flush() } /// Freezes resume invalidation so active agent sessions remain resumable across app shutdown. diff --git a/Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift b/Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift index 9c863b9..9baf967 100644 --- a/Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift +++ b/Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift @@ -98,7 +98,7 @@ final class WorkspaceManager: ObservableObject { /// Creates a manager with explicit dependencies for testability. init( - persistence: any WorkspacePersisting = WorkspacePersistence(), + persistence: (any WorkspacePersisting)? = nil, workspaceCatalog: WorkspaceCatalogManager = WorkspaceCatalogManager(), surfaceManager: WorkspaceSurfaceManager = WorkspaceSurfaceManager(), runtimeBridge: (any AgentRuntimeSupporting)? = nil, @@ -113,8 +113,10 @@ final class WorkspaceManager: ObservableObject { let resolvedRuntimeBridge = runtimeBridge ?? AgentRuntimeBridge.shared let resolvedActivityEventMonitor = activityEventMonitor ?? AgentCompletionEventMonitor(logURL: resolvedRuntimeBridge.eventLogURL) + let resolvedPersistence = persistence + ?? CoalescingWorkspacePersistence(backing: WorkspacePersistence()) - self.persistence = persistence + self.persistence = resolvedPersistence self.workspaceCatalog = workspaceCatalog self.surfaceManager = surfaceManager self.runtimeBridge = resolvedRuntimeBridge diff --git a/Tests/ShellraiserTests/CoalescingWorkspacePersistenceTests.swift b/Tests/ShellraiserTests/CoalescingWorkspacePersistenceTests.swift new file mode 100644 index 0000000..efbdb83 --- /dev/null +++ b/Tests/ShellraiserTests/CoalescingWorkspacePersistenceTests.swift @@ -0,0 +1,88 @@ +import XCTest +@testable import Shellraiser + +/// Covers debounced workspace persistence behavior and flush semantics. +@MainActor +final class CoalescingWorkspacePersistenceTests: WorkspaceTestCase { + /// Verifies repeated saves inside the debounce window collapse to one backing write. + func testSaveCoalescesRepeatedWritesAndPersistsLatestSnapshot() async { + let backing = RecordingWorkspacePersistence() + let persistence = CoalescingWorkspacePersistence(backing: backing, debounceInterval: 0.05) + let firstWorkspace = WorkspaceModel.makeDefault(name: "First") + let secondWorkspace = WorkspaceModel.makeDefault(name: "Second") + + persistence.save([firstWorkspace]) + persistence.save([secondWorkspace]) + + XCTAssertTrue(backing.savedSnapshots.isEmpty) + + try? await Task.sleep(nanoseconds: 100_000_000) + + XCTAssertEqual(backing.savedSnapshots, [[secondWorkspace]]) + XCTAssertEqual(backing.flushCallCount, 1) + } + + /// Verifies flushing bypasses the debounce delay and writes immediately. + func testFlushPersistsPendingSnapshotImmediately() { + let backing = RecordingWorkspacePersistence() + let persistence = CoalescingWorkspacePersistence(backing: backing, debounceInterval: 60) + let workspace = WorkspaceModel.makeDefault(name: "Flush") + + persistence.save([workspace]) + persistence.flush() + + XCTAssertEqual(backing.savedSnapshots, [[workspace]]) + XCTAssertEqual(backing.flushCallCount, 1) + } + + /// Verifies pending snapshots are visible to immediate reads before disk flush. + func testLoadReturnsPendingSnapshotBeforeFlush() { + let backing = RecordingWorkspacePersistence() + let persistence = CoalescingWorkspacePersistence(backing: backing, debounceInterval: 60) + let workspace = WorkspaceModel.makeDefault(name: "Pending") + + persistence.save([workspace]) + + XCTAssertEqual(persistence.load(), [workspace]) + XCTAssertTrue(backing.savedSnapshots.isEmpty) + } + + /// Verifies loads fall back to the wrapped persistence when no save is pending. + func testLoadFallsBackToBackingPersistenceWhenNoPendingSnapshot() { + let workspace = WorkspaceModel.makeDefault(name: "Persisted") + let backing = RecordingWorkspacePersistence(loadedWorkspaces: [workspace]) + let persistence = CoalescingWorkspacePersistence(backing: backing, debounceInterval: 60) + + XCTAssertEqual(persistence.load(), [workspace]) + XCTAssertEqual(backing.loadCallCount, 1) + } +} + +/// Recording persistence double used to assert debounced save behavior. +final class RecordingWorkspacePersistence: WorkspacePersisting { + private(set) var loadCallCount = 0 + private(set) var flushCallCount = 0 + private(set) var savedSnapshots: [[WorkspaceModel]] = [] + private let loadedWorkspaces: [WorkspaceModel]? + + /// Creates a recording double with an optional preloaded snapshot. + init(loadedWorkspaces: [WorkspaceModel]? = nil) { + self.loadedWorkspaces = loadedWorkspaces + } + + /// Returns the preloaded snapshot and tracks load access. + func load() -> [WorkspaceModel]? { + loadCallCount += 1 + return loadedWorkspaces + } + + /// Records each persisted workspace snapshot. + func save(_ workspaces: [WorkspaceModel]) { + savedSnapshots.append(workspaces) + } + + /// Records flush calls from the coalescing wrapper. + func flush() { + flushCallCount += 1 + } +} diff --git a/Tests/ShellraiserTests/WorkspaceManagerLifecycleTests.swift b/Tests/ShellraiserTests/WorkspaceManagerLifecycleTests.swift index 1296f6e..9a1a74c 100644 --- a/Tests/ShellraiserTests/WorkspaceManagerLifecycleTests.swift +++ b/Tests/ShellraiserTests/WorkspaceManagerLifecycleTests.swift @@ -133,4 +133,19 @@ final class WorkspaceManagerLifecycleTests: WorkspaceTestCase { XCTAssertEqual(manager.workspaces[0].focusedSurfaceId, surface.id) XCTAssertEqual(manager.workspaces[0].rootPane.firstActiveSurfaceId(), surface.id) } + + /// Verifies shutdown flushes pending debounced persistence before app teardown continues. + func testPrepareForTerminationFlushesPendingDebouncedPersistence() { + let backing = RecordingWorkspacePersistence() + let persistence = CoalescingWorkspacePersistence(backing: backing, debounceInterval: 60) + let manager = makeWorkspaceManager(persistence: persistence) + let workspace = WorkspaceModel.makeDefault(name: "Shutdown") + manager.workspaces = [workspace] + + manager.prepareForTermination() + + XCTAssertTrue(manager.isTerminating) + XCTAssertEqual(backing.savedSnapshots, [[workspace]]) + XCTAssertEqual(backing.flushCallCount, 1) + } } diff --git a/Tests/ShellraiserTests/WorkspacePersistenceTests.swift b/Tests/ShellraiserTests/WorkspacePersistenceTests.swift index 9dc3157..12c4ab2 100644 --- a/Tests/ShellraiserTests/WorkspacePersistenceTests.swift +++ b/Tests/ShellraiserTests/WorkspacePersistenceTests.swift @@ -55,6 +55,33 @@ final class WorkspacePersistenceTests: WorkspaceTestCase { XCTAssertEqual(persistence.load(), [workspace]) } + /// Verifies persisted JSON uses a compact encoding without pretty-printed whitespace. + func testSaveUsesCompactJSONEncoding() throws { + let context = makePersistenceContext() + let workspace = makeWorkspace( + id: UUID(uuidString: "00000000-0000-0000-0000-000000001041")!, + name: "Compact", + rootPane: makeLeaf( + paneId: UUID(uuidString: "00000000-0000-0000-0000-000000001042")!, + surfaces: [ + makeSurface( + id: UUID(uuidString: "00000000-0000-0000-0000-000000001043")!, + title: "Compact Surface" + ) + ] + ) + ) + + context.persistence.save([workspace]) + + let workspaceFile = context.directory.appendingPathComponent("workspaces.json") + let encodedJSON = try XCTUnwrap(String(data: Data(contentsOf: workspaceFile), encoding: .utf8)) + + XCTAssertFalse(encodedJSON.contains("\n")) + XCTAssertFalse(encodedJSON.contains(" ")) + XCTAssertTrue(encodedJSON.hasPrefix("[{")) + } + /// Verifies corrupt persistence payloads fail closed by returning nil. func testLoadReturnsNilForCorruptPersistenceFile() throws { let context = makePersistenceContext() diff --git a/Tests/ShellraiserTests/WorkspaceTestSupport.swift b/Tests/ShellraiserTests/WorkspaceTestSupport.swift index 98a7b46..a088d5d 100644 --- a/Tests/ShellraiserTests/WorkspaceTestSupport.swift +++ b/Tests/ShellraiserTests/WorkspaceTestSupport.swift @@ -165,6 +165,9 @@ final class InMemoryWorkspacePersistence: WorkspacePersisting { func save(_ workspaces: [WorkspaceModel]) { storedWorkspaces = workspaces } + + /// In-memory persistence completes writes eagerly, so flushing is a no-op. + func flush() {} } /// Minimal runtime-bridge test double for workspace-manager orchestration tests.