From 7f1fb99b67d13d1997e37775d3b2b62e87a616ee Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Sun, 27 Sep 2026 10:54:37 +0700 Subject: [PATCH 01/10] perf: streamline Welcome dashboard refreshes --- .../plans/2026-09-27-memory-cold-start.md | 112 +++++++++++++++ macgit/ViewModels/WelcomeDashboardModel.swift | 114 ++++++++++++---- macgit/Views/MainWindow/WelcomeView.swift | 40 ++++-- .../WelcomeDashboardRefreshTests.swift | 129 ++++++++++++++++++ 4 files changed, 359 insertions(+), 36 deletions(-) create mode 100644 docs/superpowers/plans/2026-09-27-memory-cold-start.md create mode 100644 macgitTests/WelcomeDashboardRefreshTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md new file mode 100644 index 0000000..d1a965f --- /dev/null +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -0,0 +1,112 @@ +# Cải thiện RAM và cold start + +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), build và 20 test pass; phần 2–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. + +Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. + +## 1. Welcome + +Điểm sửa chính: `WelcomeView`, `WelcomeDashboardModel`, `WelcomeActivityCache`. + +- [x] Hiện activity cache trước khi chờ các truy vấn Git còn lại. +- [x] Ưu tiên repo đang hiển thị/gần đây; tiếp tục kiểm tra phần còn lại ở nền, không bỏ các cảnh báo hiện có. +- [x] Gộp các yêu cầu refresh liên tiếp; tránh nhiều lượt quét chồng nhau. +- [x] Notification có repository cụ thể chỉ invalidate/cập nhật repository đó; notification không xác định repo có fallback đầy đủ. +- [x] Hủy tác vụ cũ khi yêu cầu thay đổi hoặc view đóng; ngăn kết quả cũ ghi đè kết quả mới. +- [x] Reload thủ công vẫn kiểm tra đầy đủ và bỏ qua cache phù hợp. + +Kiểm tra: nhiều repo, repo mất đường dẫn, notification dồn dập, đổi ngày, app active, Reload và đóng Welcome giữa lúc tải. Không làm mất attention của repo ngoài nhóm ưu tiên. + +## 2. AI + +Điểm sửa chính: `AIProviderController`, `macgitApp`, các màn hình chọn provider/Settings. + +- [ ] Bỏ kiểm tra toàn bộ provider từ Welcome khi không cần AI. +- [ ] Kiểm tra provider đang chọn khi mở tính năng AI; kiểm tra danh sách khi mở provider picker/Settings. +- [ ] Trì hoãn đọc credential/configuration không cần cho màn hình đầu tiên. +- [ ] Dùng chung một tác vụ refresh đang chạy; tránh lặp theo số window. +- [ ] Invalidate đúng khi đổi credential, model, account hoặc entitlement. +- [ ] Giữ kiểm tra quyền và availability tại thời điểm thực thi AI. + +Kiểm tra: guest, signed-in, đổi provider/key, nhiều window, entitlement thay đổi và gửi yêu cầu ngay khi mở tính năng. + +## 3. PR cache trên SQLite + +Điểm sửa chính: `PullRequestController`, các model PR và lifecycle provider account. + +- [ ] Tạo cache SQLite riêng trong thư mục cache của app, không dùng snapshot RAM của `LocalDataStore`. +- [ ] Đọc/ghi ngoài main thread; cache lỗi hoặc bị xóa phải fallback sang tải mạng. +- [ ] Cache list theo page; key chứa account, provider/host, repository, filter/sort và pagination. +- [ ] Cache detail riêng theo PR; changes/diff chỉ tải khi mở phần tương ứng. +- [ ] RAM chỉ giữ dữ liệu màn hình hiện tại; bỏ dictionary cache tích lũy trong controller. +- [ ] Có version schema, TTL, giới hạn tổng dung lượng disk và giới hạn mỗi entry; bỏ qua payload quá lớn. +- [ ] Dọn entry hết hạn và ít dùng; không quét/nạp toàn bộ payload để dọn cache. +- [ ] Cache hợp lệ hiển thị ngay; hết hạn refresh; Reload bỏ qua cache. +- [ ] Comment/merge/update invalidate đúng list/detail/changes liên quan. +- [ ] Xóa cache theo account khi logout/gỡ account; request cũ không được ghi lại cache sau khi xóa. +- [ ] Không lưu token/credential; không dùng dữ liệu account khác hoặc kết quả request đã lỗi thời. +- [ ] Điều chỉnh chức năng Clear Cache để bao phủ cache disk và trạng thái đang hiển thị phù hợp. + +Kiểm tra: phân trang/filter, mở lại app, TTL, offline, cache hỏng, giới hạn dung lượng, mutation, đổi account, logout trong lúc request chạy và private PR giữa hai account. + +## 4. Local storage + +Điểm sửa chính: `LocalDataStore`, `LocalSQLiteDatabase`, `LocalDataTransaction` và các store sử dụng chúng. + +- [ ] Liệt kê collection và consumer; xác định dữ liệu thật sự cần trước khi hiện màn hình đầu tiên. +- [ ] Thay đọc toàn bộ records bằng truy vấn theo collection/entity cần dùng. +- [ ] Chuyển các consumer sang tải bất đồng bộ hoặc snapshot có phạm vi rõ ràng; không đưa SQLite I/O vào main thread. +- [ ] Giữ serialization của writer và tính atomic của transaction nhiều collection. +- [ ] Giữ migration, verification, retry và thông báo lỗi; tránh mất dữ liệu khi import bị gián đoạn. +- [ ] Giải phóng snapshot không cần; tránh giải mã lại cùng dữ liệu trong mỗi lần render. + +Kiểm tra: DB mới/cũ, migration lỗi giữa chừng, ghi đồng thời, transaction lỗi, dữ liệu nhiều collection và tải lại sau ghi. Đây là phần thay đổi contract, cần review riêng. + +## 5. Cloud lifecycle + +Điểm sửa chính: `macgitApp`, `AccountSessionController`, `FeatureAccessController` và các controller sync. + +- [ ] Liệt kê dịch vụ/listener bắt đầu trong init và điều kiện thực sự cần chúng. +- [ ] Tách tạo đối tượng khỏi bắt đầu đồng bộ; trì hoãn phần không cần cho first window. +- [ ] Giữ bootstrap bắt buộc trước khi dùng Firebase API. +- [ ] Không làm yếu auth, entitlement, device enforcement hoặc feature policy trong thời gian chờ. +- [ ] Bảo đảm start idempotent; không nhân listener theo window. +- [ ] Hủy listener/task đúng khi đổi session; bỏ kết quả từ session cũ. + +Kiểm tra: guest, session được khôi phục, offline, đăng nhập/đăng xuất, đổi account, nhiều window, policy và entitlement cập nhật. Phân biệt trì hoãn công việc với giảm RAM ổn định. + +## 6. Cache và vòng đời còn lại + +- [ ] History: giới hạn snapshot theo branch/filter; giữ selection và viewport khi background refresh. +- [ ] Branch/reference cache: giới hạn entry và invalidate sau mutation. +- [ ] Diff, preview, ảnh và thumbnail: giới hạn kích thước/số entry, tải theo nhu cầu và hủy khi đóng. +- [ ] Welcome activity: kiểm tra payload commit hash, giới hạn entry và giải phóng dữ liệu không hiển thị. +- [ ] AI chat: kiểm tra conversation/context/tool output được giữ lại và vòng đời controller. +- [ ] Undo: kiểm tra giới hạn/vòng đời nhưng không xóa dữ liệu cần cho undo còn hiệu lực. +- [ ] Window/controller: kiểm tra task, timer, observer, closure và subscription giữ đối tượng sau khi đóng. +- [ ] Ghi nhận từng mục đã có giới hạn hợp lý; không refactor chỉ để thay đổi kiến trúc. + +Kiểm tra: mở/đóng nhiều repo, chuyển branch/filter, mở file lớn, chuyển PR, chat dài và undo/redo. Mỗi cache phải có owner, giới hạn, invalidation và điểm giải phóng rõ ràng. + +## 7. Kiểm tra và bàn giao + +- [ ] Mỗi phần có diff riêng dễ review, ghi rõ thay đổi và giới hạn kiểm chứng. +- [ ] Chạy coverage liên quan cho logic thay đổi; build/test tuần tự. +- [ ] Nếu XCTest abort lúc bootstrap/Firebase, không retry; báo test chưa chạy được. +- [ ] Build macOS không launch app và chạy `rtk git diff --check`. +- [ ] Đối chiếu chức năng: Welcome, AI, PR, local persistence, cloud access, History và undo. +- [ ] Ghi riêng các tương tác chưa kiểm tra runtime; build chỉ là bằng chứng biên dịch. +- [ ] Không khẳng định giảm RAM/cold start bằng con số khi chưa có phép đo thực tế. Nếu có số đo sau này, ghi rõ cấu hình và điều kiện; không dựng lại bước baseline đã bỏ. + +Không tự commit, push hoặc thay đổi release. Các checkbox chỉ được đánh dấu khi có bằng chứng hoàn thành. + +## Kết quả triển khai + +### Phần 1 — Welcome (2026-09-27) + +- Đọc cache cho toàn bộ tối đa 7 repo hiển thị trước khi bắt đầu các truy vấn activity còn thiếu; cập nhật từng repo khi tải xong. +- Attention vẫn kiểm tra toàn bộ recent repositories, ưu tiên repo mở gần đây và hiện cảnh báo dần. Kết quả đã kiểm tra được tái sử dụng đến khi invalidate. +- Notification có `repositoryURL` chỉ invalidate repo tương ứng; notification không có URL, đổi ngày và Reload invalidate đầy đủ. App active kiểm tra lại attention, giữ policy activity cache theo ngày. +- Các đợt refresh sau lần đầu được debounce 200 ms bằng task gắn với view; generation và cancellation guard chặn kết quả cũ, giữ invalidation chưa xử lý. +- Đã thêm `WelcomeDashboardRefreshTests` cho cache-first, invalidate theo repo, cảnh báo ngoài top 7, cancellation và force refresh. +- Build macOS: **PASS**. Test Welcome: **20/20 PASS**, gồm 5 test refresh mới và 15 test cache/dashboard/attention hiện có. Chưa launch/relaunch app để kiểm tra tương tác, chưa đo mức giảm RAM hoặc cold start. diff --git a/macgit/ViewModels/WelcomeDashboardModel.swift b/macgit/ViewModels/WelcomeDashboardModel.swift index 7ac5d8c..906fa99 100644 --- a/macgit/ViewModels/WelcomeDashboardModel.swift +++ b/macgit/ViewModels/WelcomeDashboardModel.swift @@ -29,28 +29,63 @@ final class WelcomeDashboardModel { private(set) var attentionUpdatedAt: Date? private var attentionGeneration = UUID() private var generation = UUID() + private var dirtyActivityURLs: Set = [] + private var checkedAttentionURLs: Set = [] + private let activityCache: WelcomeActivityCache + private let activityLoader: (RecentRepository, [Date]) async throws -> WelcomeRepositoryActivity + private let attentionLoader: (RecentRepository) async throws -> WelcomeRepositoryAttention + + init( + activityCache: WelcomeActivityCache = .shared, + activityLoader: @escaping (RecentRepository, [Date]) async throws -> WelcomeRepositoryActivity = { + try await GitStatusService.shared.welcomeActivity(for: $0, days: $1) + }, + attentionLoader: @escaping (RecentRepository) async throws -> WelcomeRepositoryAttention = { + try await GitStatusService.shared.welcomeAttention(for: $0) + } + ) { + self.activityCache = activityCache + self.activityLoader = activityLoader + self.attentionLoader = attentionLoader + } + + /// Keep invalidations until a successful read, including across cancelled refreshes. + func invalidate(repositories: [RecentRepository], activity: Bool = true) { + let urls = Set(repositories.map { $0.url.standardizedFileURL }) + if activity { dirtyActivityURLs.formUnion(urls) } + checkedAttentionURLs.subtract(urls) + generation = UUID() + attentionGeneration = UUID() + } func refreshAttention(repositories: [RecentRepository]) async { let request = UUID() attentionGeneration = request isCheckingAttention = true - var result: [WelcomeRepositoryAttention] = [] - var seen = Set() - for repository in repositories.sorted(by: { $0.lastOpened > $1.lastOpened }) - where seen.insert(repository.url.standardizedFileURL).inserted { + let recent = uniqueRepositories(repositories) + let urls = Set(recent.map { $0.url.standardizedFileURL }) + checkedAttentionURLs.formIntersection(urls) + attention.removeAll { !urls.contains($0.url.standardizedFileURL) } + for repository in recent { guard !Task.isCancelled, request == attentionGeneration else { return } + let url = repository.url.standardizedFileURL + guard !checkedAttentionURLs.contains(url) else { continue } do { - let status = try await GitStatusService.shared.welcomeAttention(for: repository) - if status.needsAttention { result.append(status) } + let status = try await attentionLoader(repository) + guard !Task.isCancelled, request == attentionGeneration else { return } + attention.removeAll { $0.url.standardizedFileURL == url } + if status.needsAttention { attention.append(status) } + attention.sort { + if $0.priority != $1.priority { return $0.priority < $1.priority } + return $0.name.localizedStandardCompare($1.name) == .orderedAscending + } + checkedAttentionURLs.insert(url) } catch { guard !Task.isCancelled, request == attentionGeneration else { return } + // Preserve the previous warning and retry on the next refresh. } } guard !Task.isCancelled, request == attentionGeneration else { return } - attention = result.sorted { - if $0.priority != $1.priority { return $0.priority < $1.priority } - return $0.name.localizedStandardCompare($1.name) == .orderedAscending - } attentionUpdatedAt = .now isCheckingAttention = false } @@ -61,31 +96,58 @@ final class WelcomeDashboardModel { isLoading = true let now = Date.now let days = WelcomeDashboardSnapshot.days(endingAt: now) - var oldestUpdate = now - var result = WelcomeDashboardSnapshot(days: days) - var seen = Set() - let recent = repositories.sorted { $0.lastOpened > $1.lastOpened } - .filter { seen.insert($0.url.standardizedFileURL).inserted } - .prefix(7) + let all = uniqueRepositories(repositories) + dirtyActivityURLs.formIntersection(Set(all.map { $0.url.standardizedFileURL })) + let recent = Array(all.prefix(7)) + if force { dirtyActivityURLs.formUnion(recent.map { $0.url.standardizedFileURL }) } + var entries: [URL: WelcomeActivityCacheEntry] = [:] + var pending: [RecentRepository] = [] + + // Read every visible cached row before starting any Git command, so a + // slow cache miss cannot hold back the other repositories' activity. for repository in recent { + let url = repository.url.standardizedFileURL + let cached = await activityCache.entry(for: url, days: days, now: now) + guard !Task.isCancelled, request == generation else { return } + if let cached { entries[url] = cached } + if cached == nil || dirtyActivityURLs.contains(url) { pending.append(repository) } + } + publishActivity(entries, repositories: recent, days: days) + for repository in pending { guard !Task.isCancelled, request == generation else { return } - if !force, let cached = await WelcomeActivityCache.shared.entry(for: repository.url, days: days, now: now) { - result.repositories.append(cached.activity) - oldestUpdate = min(oldestUpdate, cached.savedAt) - continue - } do { - let activity = try await GitStatusService.shared.welcomeActivity(for: repository, days: days) + let activity = try await activityLoader(repository, days) + guard !Task.isCancelled, request == generation else { return } + let entry = WelcomeActivityCacheEntry(days: days, savedAt: .now, activity: activity) + await activityCache.save(entry) guard !Task.isCancelled, request == generation else { return } - result.repositories.append(activity) - await WelcomeActivityCache.shared.save(WelcomeActivityCacheEntry(days: days, savedAt: now, activity: activity)) + let url = repository.url.standardizedFileURL + dirtyActivityURLs.remove(url) + entries[url] = entry + publishActivity(entries, repositories: recent, days: days) } catch { guard !Task.isCancelled, request == generation else { return } } } guard !Task.isCancelled, request == generation else { return } - snapshot = result - updatedAt = oldestUpdate isLoading = false } + + private func publishActivity( + _ entries: [URL: WelcomeActivityCacheEntry], + repositories: [RecentRepository], + days: [Date] + ) { + var result = WelcomeDashboardSnapshot(days: days) + let ordered = repositories.compactMap { entries[$0.url.standardizedFileURL] } + result.repositories = ordered.map(\.activity) + snapshot = result + updatedAt = ordered.map(\.savedAt).min() + } + + private func uniqueRepositories(_ repositories: [RecentRepository]) -> [RecentRepository] { + var seen = Set() + return repositories.sorted { $0.lastOpened > $1.lastOpened } + .filter { seen.insert($0.url.standardizedFileURL).inserted } + } } diff --git a/macgit/Views/MainWindow/WelcomeView.swift b/macgit/Views/MainWindow/WelcomeView.swift index 49b3011..4335424 100644 --- a/macgit/Views/MainWindow/WelcomeView.swift +++ b/macgit/Views/MainWindow/WelcomeView.swift @@ -24,7 +24,7 @@ struct WelcomeView: View { @ObservedObject private var store = RecentRepositoriesStore.shared @State private var model = WelcomeDashboardModel() @State private var showingUnavailableRepository = false - @State private var forceRefresh = false + @State private var hasStartedLoading = false @State private var refreshID = UUID() let accountDisplayName: String? let onOpenAccount: () -> Void @@ -53,16 +53,25 @@ struct WelcomeView: View { Text(locationError ?? "Use the repository list to locate its folder or remove it from recents.") } .task(id: refreshID) { - let force = forceRefresh - forceRefresh = false - await model.refresh(repositories: store.repositories, force: force) + // The first render uses disk cache immediately. Subsequent events + // share a short cancellable debounce instead of starting Git scans. + if hasStartedLoading { + do { try await Task.sleep(for: .milliseconds(200)) } + catch { return } + } + hasStartedLoading = true + async let activity: Void = model.refresh(repositories: store.repositories) + async let attention: Void = model.refreshAttention(repositories: store.repositories) + _ = await (activity, attention) } - .task(id: refreshID) { await model.refreshAttention(repositories: store.repositories) } .onChange(of: store.repositories.map { "\($0.url.path)|\($0.lastOpened.timeIntervalSince1970)" }) { _, _ in refresh() } - .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in refresh() } - .onReceive(NotificationCenter.default.publisher(for: .NSCalendarDayChanged)) { _ in refresh() } - .onReceive(NotificationCenter.default.publisher(for: .repositoryDidChange)) { _ in refresh() } - .onReceive(NotificationCenter.default.publisher(for: .repositoryLocalStateDidRefresh)) { _ in refresh() } + .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in + model.invalidate(repositories: store.repositories, activity: false) + refresh() + } + .onReceive(NotificationCenter.default.publisher(for: .NSCalendarDayChanged)) { _ in refreshImmediately() } + .onReceive(NotificationCenter.default.publisher(for: .repositoryDidChange), perform: refreshRepository) + .onReceive(NotificationCenter.default.publisher(for: .repositoryLocalStateDidRefresh), perform: refreshRepository) } private func reviewAttention(_ repository: WelcomeRepositoryAttention) { @@ -113,7 +122,18 @@ struct WelcomeView: View { } private func refreshImmediately() { - forceRefresh = true + model.invalidate(repositories: store.repositories) + refresh() + } + + private func refreshRepository(_ notification: Notification) { + if let url = notification.userInfo?["repositoryURL"] as? URL { + let repositories = store.repositories.filter { $0.url.standardizedFileURL == url.standardizedFileURL } + guard !repositories.isEmpty else { return } + model.invalidate(repositories: repositories) + } else { + model.invalidate(repositories: store.repositories) + } refresh() } diff --git a/macgitTests/WelcomeDashboardRefreshTests.swift b/macgitTests/WelcomeDashboardRefreshTests.swift new file mode 100644 index 0000000..d6b8d18 --- /dev/null +++ b/macgitTests/WelcomeDashboardRefreshTests.swift @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +@MainActor +final class WelcomeDashboardRefreshTests: XCTestCase { + func testCachedRowsArePublishedBeforeFirstGitRead() async { + let cache = WelcomeActivityCache(fileURL: nil) + let missing = RecentRepository(url: URL(fileURLWithPath: "/missing")) + let cached = RecentRepository(url: URL(fileURLWithPath: "/cached")) + let days = WelcomeDashboardSnapshot.days(endingAt: .now) + await cache.save(WelcomeActivityCacheEntry(days: days, savedAt: .now, + activity: activity(cached, days))) + var model: WelcomeDashboardModel! + defer { model = nil } + var loaded: [URL] = [] + model = WelcomeDashboardModel(activityCache: cache, activityLoader: { repository, days in + loaded.append(repository.url) + XCTAssertEqual(model.snapshot.repositories.map(\.url), [cached.url]) + return self.activity(repository, days) + }) + await model.refresh(repositories: [missing, cached]) + XCTAssertEqual(loaded, [missing.url]) + XCTAssertEqual(model.snapshot.repositories.count, 2) + XCTAssertFalse(model.isLoading) + } + + func testTargetedInvalidationPreservesOtherActivityAndWarnings() async { + let first = RecentRepository(url: URL(fileURLWithPath: "/first")) + let second = RecentRepository(url: URL(fileURLWithPath: "/second")) + var activityReads: [URL] = [] + var attentionReads: [URL] = [] + let model = WelcomeDashboardModel(activityCache: WelcomeActivityCache(fileURL: nil), + activityLoader: { repository, days in + activityReads.append(repository.url) + return self.activity(repository, days) + }, attentionLoader: { repository in + attentionReads.append(repository.url) + var status = WelcomeRepositoryAttention(url: repository.url, name: repository.name) + status.conflicts = 1 + return status + }) + let repositories = [first, second] + await model.refresh(repositories: repositories) + await model.refreshAttention(repositories: repositories) + activityReads.removeAll() + attentionReads.removeAll() + model.invalidate(repositories: [first]) + await model.refresh(repositories: repositories) + await model.refreshAttention(repositories: repositories) + XCTAssertEqual(activityReads, [first.url]) + XCTAssertEqual(attentionReads, [first.url]) + XCTAssertEqual(Set(model.attention.map(\.url)), Set(repositories.map(\.url))) + XCTAssertEqual(model.snapshot.repositories.count, 2) + } + + func testAttentionIncludesRepositoriesOutsideActivityLimitAndPrunesRemovedOnes() async { + let repositories = (0..<10).map { RecentRepository(url: URL(fileURLWithPath: "/repo-\($0)")) } + var reads = 0 + let model = WelcomeDashboardModel(attentionLoader: { repository in + reads += 1 + var status = WelcomeRepositoryAttention(url: repository.url, name: repository.name) + status.behind = 1 + return status + }) + await model.refreshAttention(repositories: repositories) + XCTAssertEqual(reads, 10) + XCTAssertEqual(model.attention.count, 10) + await model.refreshAttention(repositories: Array(repositories.prefix(2))) + XCTAssertEqual(reads, 10) + XCTAssertEqual(model.attention.count, 2) + model.invalidate(repositories: Array(repositories.prefix(2)), activity: false) + await model.refreshAttention(repositories: Array(repositories.prefix(2))) + XCTAssertEqual(reads, 12) + } + + func testCancelledReadCannotPublishOrClearPendingInvalidation() async { + let repository = RecentRepository(url: URL(fileURLWithPath: "/repo")) + let cache = WelcomeActivityCache(fileURL: nil) + let days = WelcomeDashboardSnapshot.days(endingAt: .now) + var cachedActivity = activity(repository, days) + cachedActivity.activityNote = "Cached" + await cache.save(WelcomeActivityCacheEntry(days: days, savedAt: .now, activity: cachedActivity)) + let started = expectation(description: "Activity read started") + var continuation: CheckedContinuation? + var reads = 0 + let model = WelcomeDashboardModel(activityCache: cache, + activityLoader: { repository, days in + reads += 1 + if reads == 1 { + return await withCheckedContinuation { + continuation = $0 + started.fulfill() + } + } + return self.activity(repository, days) + }) + model.invalidate(repositories: [repository]) + let task = Task { await model.refresh(repositories: [repository]) } + await fulfillment(of: [started], timeout: 2) + task.cancel() + continuation?.resume(returning: activity(repository, WelcomeDashboardSnapshot.days(endingAt: .now))) + await task.value + XCTAssertEqual(model.snapshot.repositories.first?.activityNote, "Cached") + await model.refresh(repositories: [repository]) + XCTAssertEqual(reads, 2) + XCTAssertEqual(model.snapshot.repositories.count, 1) + XCTAssertNil(model.snapshot.repositories.first?.activityNote) + } + + func testForceRefreshBypassesCache() async { + let repository = RecentRepository(url: URL(fileURLWithPath: "/repo")) + var reads = 0 + let model = WelcomeDashboardModel(activityCache: WelcomeActivityCache(fileURL: nil), + activityLoader: { repository, days in + reads += 1 + return self.activity(repository, days) + }) + await model.refresh(repositories: [repository]) + await model.refresh(repositories: [repository]) + XCTAssertEqual(reads, 1) + await model.refresh(repositories: [repository], force: true) + XCTAssertEqual(reads, 2) + } + + private func activity(_ repository: RecentRepository, _ days: [Date]) -> WelcomeRepositoryActivity { + WelcomeRepositoryActivity(url: repository.url, name: repository.name, commitsByDay: days.map { _ in [] }) + } +} From b10ad73071eb6a44e852d79d4103c865845159a7 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Sun, 27 Sep 2026 11:00:25 +0700 Subject: [PATCH 02/10] perf: Defer AI provider availability checks Read API keys on demand and share availability tasks per provider. Invalidate availability instead of refreshing all providers from Welcome. Refresh visible AI surfaces by revision and skip managed usage unless requested. --- .../plans/2026-09-27-memory-cold-start.md | 25 ++-- macgit/App/AIProviderController.swift | 66 ++++++++-- macgit/App/macgitApp.swift | 8 +- .../Common/AIProvidersSettingsView.swift | 2 +- .../Views/Common/ConflictMergeToolView.swift | 2 +- macgit/Views/FileStatus/AIProviderMenu.swift | 3 + macgit/Views/FileStatus/CommitSheetView.swift | 2 +- macgit/Views/FileStatus/FileStatusView.swift | 2 +- .../MainWindow/RepositoryAIChatView.swift | 2 +- macgitTests/AIProviderAvailabilityTests.swift | 115 ++++++++++++++++++ macgitTests/CloudAIProviderTests.swift | 3 +- 11 files changed, 201 insertions(+), 29 deletions(-) create mode 100644 macgitTests/AIProviderAvailabilityTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index d1a965f..eb6646b 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -1,6 +1,6 @@ # Cải thiện RAM và cold start -Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), build và 20 test pass; phần 2–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome) và phần 2 (AI). Phần 3–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. @@ -21,12 +21,12 @@ Kiểm tra: nhiều repo, repo mất đường dẫn, notification dồn dập, Điểm sửa chính: `AIProviderController`, `macgitApp`, các màn hình chọn provider/Settings. -- [ ] Bỏ kiểm tra toàn bộ provider từ Welcome khi không cần AI. -- [ ] Kiểm tra provider đang chọn khi mở tính năng AI; kiểm tra danh sách khi mở provider picker/Settings. -- [ ] Trì hoãn đọc credential/configuration không cần cho màn hình đầu tiên. -- [ ] Dùng chung một tác vụ refresh đang chạy; tránh lặp theo số window. -- [ ] Invalidate đúng khi đổi credential, model, account hoặc entitlement. -- [ ] Giữ kiểm tra quyền và availability tại thời điểm thực thi AI. +- [x] Bỏ kiểm tra toàn bộ provider từ Welcome khi không cần AI. +- [ ] Kiểm tra provider đang chọn khi mở tính năng AI; kiểm tra danh sách khi mở provider picker/Settings. Đã tách theo màn hình; danh sách hiện được kiểm tra khi control picker xuất hiện, chưa phải chỉ khi dropdown mở. +- [x] Trì hoãn đọc credential/configuration không cần cho màn hình đầu tiên. +- [x] Dùng chung một tác vụ refresh đang chạy; tránh lặp theo số window. +- [x] Invalidate đúng khi đổi credential, model, account hoặc entitlement. +- [x] Giữ kiểm tra quyền và availability tại thời điểm thực thi AI. Kiểm tra: guest, signed-in, đổi provider/key, nhiều window, entitlement thay đổi và gửi yêu cầu ngay khi mở tính năng. @@ -110,3 +110,14 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Các đợt refresh sau lần đầu được debounce 200 ms bằng task gắn với view; generation và cancellation guard chặn kết quả cũ, giữ invalidation chưa xử lý. - Đã thêm `WelcomeDashboardRefreshTests` cho cache-first, invalidate theo repo, cảnh báo ngoài top 7, cancellation và force refresh. - Build macOS: **PASS**. Test Welcome: **20/20 PASS**, gồm 5 test refresh mới và 15 test cache/dashboard/attention hiện có. Chưa launch/relaunch app để kiểm tra tương tác, chưa đo mức giảm RAM hoặc cold start. + +### Phần 2 — AI (2026-09-27) + +- Commit phần 1 trên branch `feature/improve-memory-and-app`: `7f1fb99` (`perf: streamline Welcome dashboard refreshes`). +- Bỏ đọc API key trong init và bỏ refresh toàn bộ provider từ Welcome khi khởi tạo/đổi account/entitlement. +- Các màn hình dùng AI yêu cầu availability của selection; provider menu khi xuất hiện và AI Settings yêu cầu toàn bộ danh sách để các lựa chọn có trạng thái đúng. Chưa chuyển việc kiểm tra danh sách sang đúng sự kiện dropdown mở; hiện gắn với vòng đời control hiển thị. +- Giữ đọc tên model từ UserDefaults để nhãn model và draft Settings đúng ngay khi hiện; không mở Keychain cho việc này. +- Các request availability đồng thời dùng chung task theo provider. Revision/identity loại kết quả cũ sau đổi account, entitlement, key hoặc model; UI AI đang hiển thị refresh theo revision. +- App active chỉ kiểm tra managed usage nếu provider đó từng được yêu cầu và session còn quyền sử dụng. Kiểm tra quyền khi thực thi AI được giữ nguyên. +- Build macOS: **PASS**. Các nhóm AIProviderAvailability, AICommitMessage, CloudAIProvider và CommitPlusAIUsageController: **44/44 PASS**. +- Chưa kiểm tra tương tác dropdown/nhiều window bằng runtime; chưa đo giảm RAM/cold start. Phần 2 hiện chưa commit. diff --git a/macgit/App/AIProviderController.swift b/macgit/App/AIProviderController.swift index a41748d..bbfa6a3 100644 --- a/macgit/App/AIProviderController.swift +++ b/macgit/App/AIProviderController.swift @@ -26,6 +26,10 @@ final class AIProviderController: ObservableObject { @Published private(set) var isGenerating = false @Published private(set) var selectedProviderID: AIProviderID + @Published private(set) var availabilityRevision = 0 + private var availabilityTasks: [AIProviderID: (id: UUID, task: Task)] = [:] + private var requestedProviderIDs: Set = [] + let managedUsageController: CommitPlusAIUsageController? private var usageObservation: AnyCancellable? private let managedProviderAccess: () -> Bool @@ -96,10 +100,6 @@ final class AIProviderController: ObservableObject { for descriptor in registry.descriptors { availabilityByProviderID[descriptor.id] = descriptor.isImplemented ? .checking : .comingSoon - if descriptor.billing == .bringYourOwnKey, - (try? credentialStore.apiKey(for: descriptor.id)) != nil { - configuredProviderIDs.insert(descriptor.id) - } if descriptor.billing == .bringYourOwnKey, let customModel = modelStore.customModel(for: descriptor.id) { customModelsByProviderID[descriptor.id] = customModel @@ -144,7 +144,7 @@ final class AIProviderController: ObservableObject { } selectedProviderID = id defaults.set(id.rawValue, forKey: selectedProviderDefaultsKey) - if descriptor.billing == .commitPlus { Task { await managedUsageController?.refresh() } } + Task { await refreshAvailability(for: id) } } func isAPIKeyConfigured(for id: AIProviderID) -> Bool { @@ -196,6 +196,7 @@ final class AIProviderController: ObservableObject { } try credentialStore.saveAPIKey(normalizedKey, for: id) configuredProviderIDs.insert(id) + invalidateAvailability() } func removeAPIKey(for id: AIProviderID) throws { @@ -204,6 +205,7 @@ final class AIProviderController: ObservableObject { } try credentialStore.deleteAPIKey(for: id) configuredProviderIDs.remove(id) + invalidateAvailability() if selectedProviderID == id { selectProvider(.appleIntelligence) } @@ -244,6 +246,7 @@ final class AIProviderController: ObservableObject { customModelsByProviderID[draft.id] = normalizedModel } } + if !drafts.isEmpty { invalidateAvailability() } } private func validateProviderAccess(_ descriptor: AIProviderDescriptor) throws { @@ -260,18 +263,55 @@ final class AIProviderController: ObservableObject { } } - func refreshAvailability() async { - await withTaskGroup(of: (AIProviderID, AIProviderAvailability).self) { group in - for provider in registry.providers { - let id = provider.descriptor.id - group.addTask { (id, await provider.availability()) } - } - for await (id, availability) in group { - availabilityByProviderID[id] = availability + /// Invalidating is cheap and does not start provider work from the Welcome window. + /// Visible AI surfaces observe the revision and request their own refresh. + func invalidateAvailability() { + for request in availabilityTasks.values { request.task.cancel() } + availabilityTasks.removeAll() + for descriptor in registry.descriptors { + availabilityByProviderID[descriptor.id] = descriptor.isImplemented ? .checking : .comingSoon + } + availabilityRevision += 1 + } + + func refreshManagedUsageIfNeeded() async { + guard requestedProviderIDs.contains(.commitPlusAI), managedProviderAccess() else { return } + await refreshAvailability(for: .commitPlusAI) + } + + func refreshAvailability(selectedOnly: Bool = false) async { + let ids = selectedOnly ? [selectedProviderID] : registry.providers.map { $0.descriptor.id } + await withTaskGroup(of: Void.self) { group in + for id in ids { + group.addTask { await self.refreshAvailability(for: id) } } } } + private func refreshAvailability(for id: AIProviderID) async { + guard !Task.isCancelled, let provider = registry.provider(for: id) else { return } + requestedProviderIDs.insert(id) + let request: (id: UUID, task: Task) + if let pending = availabilityTasks[id] { + request = pending + } else { + if provider.descriptor.billing == .bringYourOwnKey { + if (try? credentialStore.apiKey(for: id)) != nil { + configuredProviderIDs.insert(id) + } else { + configuredProviderIDs.remove(id) + } + } + request = (UUID(), Task { await provider.availability() }) + availabilityTasks[id] = request + } + let value = await request.task.value + // A changed account/key or a newer refresh must win over an old result. + guard availabilityTasks[id]?.id == request.id else { return } + availabilityTasks[id] = nil + availabilityByProviderID[id] = value + } + func generateCommitMessage( repositoryURL: URL, branchName: String?, diff --git a/macgit/App/macgitApp.swift b/macgit/App/macgitApp.swift index 2f5f48b..987026b 100644 --- a/macgit/App/macgitApp.swift +++ b/macgit/App/macgitApp.swift @@ -230,14 +230,16 @@ struct macgitApp: App { } .onChange(of: accountController.account?.uid, initial: true) { _, uid in aiProviderController.managedUsageController?.setSession(uid: uid) - Task { await aiProviderController.refreshAvailability() } + } + .onChange(of: accountController.account?.uid) { _, _ in + aiProviderController.invalidateAvailability() } .onChange(of: accountController.entitlement) { _, _ in - Task { await aiProviderController.refreshAvailability() } + aiProviderController.invalidateAvailability() } .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in Task { - await aiProviderController.managedUsageController?.refresh() + await aiProviderController.refreshManagedUsageIfNeeded() } } } diff --git a/macgit/Views/Common/AIProvidersSettingsView.swift b/macgit/Views/Common/AIProvidersSettingsView.swift index 7d3c4b6..9b6f6fe 100644 --- a/macgit/Views/Common/AIProvidersSettingsView.swift +++ b/macgit/Views/Common/AIProvidersSettingsView.swift @@ -79,7 +79,7 @@ struct AIProvidersSettingsView: View { authenticationMode = nil } } - .task { + .task(id: controller.availabilityRevision) { await controller.refreshAvailability() } } diff --git a/macgit/Views/Common/ConflictMergeToolView.swift b/macgit/Views/Common/ConflictMergeToolView.swift index dc2795b..24ec9f6 100644 --- a/macgit/Views/Common/ConflictMergeToolView.swift +++ b/macgit/Views/Common/ConflictMergeToolView.swift @@ -97,7 +97,7 @@ struct ConflictMergeToolView: View { await loadMergeContext() } .task { - await aiProviderController.refreshAvailability() + await aiProviderController.refreshAvailability(selectedOnly: true) } .alert("Error", isPresented: $showingError, actions: { Button("OK", role: .cancel) {} diff --git a/macgit/Views/FileStatus/AIProviderMenu.swift b/macgit/Views/FileStatus/AIProviderMenu.swift index 6e31f19..ba08d8b 100644 --- a/macgit/Views/FileStatus/AIProviderMenu.swift +++ b/macgit/Views/FileStatus/AIProviderMenu.swift @@ -84,6 +84,9 @@ struct AIProviderMenu: View { .buttonStyle(GlassButtonStyle(tint: .secondary, fontSize: 10)) .disabled(controller.isGenerating) .help(providerHelp) + .task(id: controller.availabilityRevision) { + await controller.refreshAvailability() + } } private var labelTitle: String { diff --git a/macgit/Views/FileStatus/CommitSheetView.swift b/macgit/Views/FileStatus/CommitSheetView.swift index db370fb..acdef0f 100644 --- a/macgit/Views/FileStatus/CommitSheetView.swift +++ b/macgit/Views/FileStatus/CommitSheetView.swift @@ -99,7 +99,7 @@ struct CommitSheetView: View { .padding(30) .frame(minWidth: 480) .task { - await aiProviderController.refreshAvailability() + await aiProviderController.refreshAvailability(selectedOnly: true) } .alert("Unable to Generate Commit Message", isPresented: $showingError) { Button("OK", role: .cancel) {} diff --git a/macgit/Views/FileStatus/FileStatusView.swift b/macgit/Views/FileStatus/FileStatusView.swift index 88eb950..1e07b73 100644 --- a/macgit/Views/FileStatus/FileStatusView.swift +++ b/macgit/Views/FileStatus/FileStatusView.swift @@ -1109,7 +1109,7 @@ struct FileStatusView: View { .padding(.horizontal, 16) .padding(.vertical, 10) .task { - await aiProviderController.refreshAvailability() + await aiProviderController.refreshAvailability(selectedOnly: true) } .aiCommitMessageAccessGate( isRequested: $isAIGenerationRequested, diff --git a/macgit/Views/MainWindow/RepositoryAIChatView.swift b/macgit/Views/MainWindow/RepositoryAIChatView.swift index 51c8b25..a843e47 100644 --- a/macgit/Views/MainWindow/RepositoryAIChatView.swift +++ b/macgit/Views/MainWindow/RepositoryAIChatView.swift @@ -95,7 +95,7 @@ struct RepositoryAIChatView: View { } .padding(16) .task { - await providerController.refreshAvailability() + await providerController.refreshAvailability(selectedOnly: true) } .replacingSheet(isPresented: $isShowingHistory) { RepositoryAIChatHistorySheet(controller: controller) diff --git a/macgitTests/AIProviderAvailabilityTests.swift b/macgitTests/AIProviderAvailabilityTests.swift new file mode 100644 index 0000000..4423443 --- /dev/null +++ b/macgitTests/AIProviderAvailabilityTests.swift @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +@MainActor +final class AIProviderAvailabilityTests: XCTestCase { + func testInitializationDoesNotReadCredentials() async { + let credentials = AvailabilityCredentialStore() + let controller = makeController(registry: .live(credentialStore: credentials), credentials: credentials) + XCTAssertEqual(credentials.readCount, 0) + XCTAssertEqual(controller.selectedProviderAvailability, .checking) + controller.invalidateAvailability() + XCTAssertEqual(credentials.readCount, 0) + await controller.refreshAvailability() + XCTAssertGreaterThan(credentials.readCount, 0) + XCTAssertTrue(controller.isAPIKeyConfigured(for: .openAI)) + } + + func testSelectedRefreshDoesNotCheckOtherProviders() async { + let apple = AvailabilityProbe() + let cloud = AvailabilityProbe() + let controller = makeController(registry: AIProviderRegistry(providers: [ + AvailabilityTestProvider(id: .appleIntelligence, probe: apple), + AvailabilityTestProvider(id: .openAI, probe: cloud), + ])) + await controller.refreshAvailability(selectedOnly: true) + let appleCalls = await apple.calls + let cloudCalls = await cloud.calls + XCTAssertEqual(appleCalls, 1) + XCTAssertEqual(cloudCalls, 0) + } + + func testInvalidationRejectsSuspendedOldResult() async { + let started = expectation(description: "Old availability read") + let probe = AvailabilityProbe(started: { started.fulfill() }) + let controller = makeController(registry: AIProviderRegistry(providers: [ + AvailabilityTestProvider(id: .appleIntelligence, probe: probe) + ])) + let first = Task { await controller.refreshAvailability() } + await fulfillment(of: [started], timeout: 2) + controller.invalidateAvailability() + await controller.refreshAvailability() + XCTAssertEqual(controller.selectedProviderAvailability, .available) + await probe.finishOldRead() + await first.value + XCTAssertEqual(controller.selectedProviderAvailability, .available) + let calls = await probe.calls + XCTAssertEqual(calls, 2) + } + + private func makeController( + registry: AIProviderRegistry, + credentials: any AIProviderCredentialStore = AvailabilityCredentialStore() + ) -> AIProviderController { + let suite = "AIProviderAvailabilityTests.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suite)! + defaults.removePersistentDomain(forName: suite) + return AIProviderController(registry: registry, snapshotLoader: GitStatusService.shared, + defaults: defaults, credentialStore: credentials) + } +} + +private actor AvailabilityProbe { + private(set) var calls = 0 + private var continuation: CheckedContinuation? + private let started: (@Sendable () -> Void)? + + init(started: (@Sendable () -> Void)? = nil) { self.started = started } + + func read() async -> AIProviderAvailability { + calls += 1 + if calls == 1, let started { + return await withCheckedContinuation { + continuation = $0 + started() + } + } + return .available + } + + func finishOldRead() { + continuation?.resume(returning: .unavailable("Old account result")) + continuation = nil + } +} + +private struct AvailabilityTestProvider: CommitMessageAIProvider { + let descriptor: AIProviderDescriptor + let probe: AvailabilityProbe + + init(id: AIProviderID, probe: AvailabilityProbe) { + descriptor = AIProviderDescriptor(id: id, displayName: "Test", systemImage: "sparkles", + detail: "Test", dataProcessing: .onDevice, billing: .none, + requiresProToConfigureAPIKey: false, defaultModel: nil, + inputCharacterBudget: 100, isImplemented: true) + self.probe = probe + } + + func availability() async -> AIProviderAvailability { await probe.read() } + func generateCommitMessage(request: CommitMessageGenerationRequest) async throws -> GeneratedCommitMessage { + GeneratedCommitMessage(subject: "Test", body: "") + } +} + +private final class AvailabilityCredentialStore: AIProviderCredentialStore, @unchecked Sendable { + private let lock = NSLock() + private var reads = 0 + var readCount: Int { lock.withLock { reads } } + func apiKey(for providerID: AIProviderID) throws -> String? { + lock.withLock { reads += 1 } + return providerID == .openAI ? "test-key" : nil + } + func saveAPIKey(_ apiKey: String, for providerID: AIProviderID) throws {} + func deleteAPIKey(for providerID: AIProviderID) throws {} +} diff --git a/macgitTests/CloudAIProviderTests.swift b/macgitTests/CloudAIProviderTests.swift index 9af3d96..fb2a31f 100644 --- a/macgitTests/CloudAIProviderTests.swift +++ b/macgitTests/CloudAIProviderTests.swift @@ -154,7 +154,7 @@ final class CloudAIProviderTests: XCTestCase { } @MainActor - func testControllerLeavesConfiguredKeyUnchangedWhenDraftIsEmpty() throws { + func testControllerLeavesConfiguredKeyUnchangedWhenDraftIsEmpty() async throws { let credentialStore = InMemoryAIProviderCredentialStore(keys: [ .openAI: "existing-openai-key", ]) @@ -169,6 +169,7 @@ final class CloudAIProviderTests: XCTestCase { AIProviderConfigurationDraft(id: .openAI), ], restrictedProviderAccess: .allowed) + await controller.refreshAvailability() XCTAssertEqual(try credentialStore.apiKey(for: .openAI), "existing-openai-key") XCTAssertTrue(controller.isAPIKeyConfigured(for: .openAI)) } From 6766ad43d0d4f7b675447bff9fb2710b585225c0 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Sun, 27 Sep 2026 17:48:33 +0700 Subject: [PATCH 03/10] perf: Add PR disk cache and remove RAM caches Introduce PullRequestDiskCache actor for SQLite-backed PR list, detail, and changes caching off the main thread. Drop in-memory dictionary caches from PullRequestController and release screen data on exit. Invalidate and clear disk cache on account removal, logout, and Clear Cache; guard stale responses with load IDs and epochs. --- .../plans/2026-09-27-memory-cold-start.md | 38 ++-- macgit/App/GitProviderAccountController.swift | 9 + macgit/App/PullRequestController.swift | 203 ++++++++++++------ macgit/Models/PullRequestChangedFile.swift | 4 +- macgit/Models/PullRequestModels.swift | 20 +- macgit/Services/Commit.swift | 2 +- macgit/Services/PullRequestDiskCache.swift | 144 +++++++++++++ .../Views/Common/AdvancedSettingsView.swift | 1 + .../PullRequests/PullRequestListView.swift | 8 + macgitTests/PullRequestControllerTests.swift | 82 ++++++- macgitTests/PullRequestDiskCacheTests.swift | 92 ++++++++ 11 files changed, 504 insertions(+), 99 deletions(-) create mode 100644 macgit/Services/PullRequestDiskCache.swift create mode 100644 macgitTests/PullRequestDiskCacheTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index eb6646b..97a49c7 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -1,6 +1,6 @@ # Cải thiện RAM và cold start -Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome) và phần 2 (AI). Phần 3–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown) và phần 3 (PR cache). Phần 4–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. @@ -34,18 +34,18 @@ Kiểm tra: guest, signed-in, đổi provider/key, nhiều window, entitlement t Điểm sửa chính: `PullRequestController`, các model PR và lifecycle provider account. -- [ ] Tạo cache SQLite riêng trong thư mục cache của app, không dùng snapshot RAM của `LocalDataStore`. -- [ ] Đọc/ghi ngoài main thread; cache lỗi hoặc bị xóa phải fallback sang tải mạng. -- [ ] Cache list theo page; key chứa account, provider/host, repository, filter/sort và pagination. -- [ ] Cache detail riêng theo PR; changes/diff chỉ tải khi mở phần tương ứng. -- [ ] RAM chỉ giữ dữ liệu màn hình hiện tại; bỏ dictionary cache tích lũy trong controller. -- [ ] Có version schema, TTL, giới hạn tổng dung lượng disk và giới hạn mỗi entry; bỏ qua payload quá lớn. -- [ ] Dọn entry hết hạn và ít dùng; không quét/nạp toàn bộ payload để dọn cache. -- [ ] Cache hợp lệ hiển thị ngay; hết hạn refresh; Reload bỏ qua cache. -- [ ] Comment/merge/update invalidate đúng list/detail/changes liên quan. -- [ ] Xóa cache theo account khi logout/gỡ account; request cũ không được ghi lại cache sau khi xóa. -- [ ] Không lưu token/credential; không dùng dữ liệu account khác hoặc kết quả request đã lỗi thời. -- [ ] Điều chỉnh chức năng Clear Cache để bao phủ cache disk và trạng thái đang hiển thị phù hợp. +- [x] Tạo cache SQLite riêng trong thư mục cache của app, không dùng snapshot RAM của `LocalDataStore`. +- [x] Đọc/ghi ngoài main thread; cache lỗi hoặc bị xóa phải fallback sang tải mạng. +- [x] Cache list theo page; key chứa account, provider/host, repository, filter/sort và pagination. +- [x] Cache detail riêng theo PR; changes/diff chỉ tải khi mở phần tương ứng. +- [x] RAM chỉ giữ dữ liệu màn hình hiện tại; bỏ dictionary cache tích lũy trong controller. +- [x] Có version schema, TTL, giới hạn tổng dung lượng disk và giới hạn mỗi entry; bỏ qua payload quá lớn. +- [x] Dọn entry hết hạn và ít dùng; không quét/nạp toàn bộ payload để dọn cache. +- [x] Cache hợp lệ hiển thị ngay; hết hạn refresh; Reload bỏ qua cache. +- [x] Comment/merge/update invalidate đúng list/detail/changes liên quan. +- [x] Xóa cache theo account khi logout/gỡ account; request cũ không được ghi lại cache sau khi xóa. +- [x] Không lưu token/credential; không dùng dữ liệu account khác hoặc kết quả request đã lỗi thời. +- [x] Điều chỉnh chức năng Clear Cache để bao phủ cache disk và trạng thái đang hiển thị phù hợp. Kiểm tra: phân trang/filter, mở lại app, TTL, offline, cache hỏng, giới hạn dung lượng, mutation, đổi account, logout trong lúc request chạy và private PR giữa hai account. @@ -121,3 +121,15 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - App active chỉ kiểm tra managed usage nếu provider đó từng được yêu cầu và session còn quyền sử dụng. Kiểm tra quyền khi thực thi AI được giữ nguyên. - Build macOS: **PASS**. Các nhóm AIProviderAvailability, AICommitMessage, CloudAIProvider và CommitPlusAIUsageController: **44/44 PASS**. - Chưa kiểm tra tương tác dropdown/nhiều window bằng runtime; chưa đo giảm RAM/cold start. Phần 2 hiện chưa commit. + +### Phần 3 — PR cache SQLite (2026-09-27) + +- `PullRequestDiskCache` là actor riêng, đọc/ghi/encode/decode SQLite ngoài main actor, không dùng `LocalDataStore` và không giữ dictionary payload trong RAM. +- List/detail/changes được cache theo account, provider, host, repository, loại dữ liệu và page/filter hoặc số PR. Sort của provider hiện cố định; created-by-me là filter trên page đang hiển thị. +- TTL giữ như trước: list 120 giây, detail/changes 300 giây. Giới hạn 32 MiB payload, 300 entry, 2 MiB/entry; SQLite tự thu hồi page và giới hạn 10.240 page. Payload quá lớn vẫn trả về UI, không lưu cache. +- Dọn hết hạn và entry ít dùng bằng metadata SQL; cache lỗi là cache miss. Clear Cache/logout có thể xóa cache hỏng. File cache có permission 0600; không serialize token. +- Bỏ ba dictionary cache trong PR controller. View giải phóng list/detail/changes khi rời màn hình, lần sau đọc lại disk; request ID chặn response từ selection/view/account cũ. Epoch trong cache chặn ghi lại dữ liệu sau invalidation. +- Comment/create/merge giữ invalidation và force-refresh hiện có. Clear Cache bao gồm SQLite kể cả khi không có cửa sổ repo; màn hình PR đang mở tải lại từ mạng. +- Gỡ provider account xóa cache tương ứng. Logout/đổi tài khoản Commit+ xóa cache phiên trước; khôi phục auth ban đầu không xóa cache hợp lệ. Tests dùng DB tạm riêng. +- Đã kiểm tra persistence qua cache/controller mới, TTL, giới hạn/oversize, corruption, namespace account/repo/filter/page và response đến muộn sau Clear Cache. +- Validation cuối trên mã bàn giao: **BUILD SUCCEEDED**, **85/85 test PASS**, `git diff --check` sạch. Không đo baseline, không tự mở lại app để kiểm tra UI. Chưa commit phần 3. diff --git a/macgit/App/GitProviderAccountController.swift b/macgit/App/GitProviderAccountController.swift index 197cce3..9c97f9e 100644 --- a/macgit/App/GitProviderAccountController.swift +++ b/macgit/App/GitProviderAccountController.swift @@ -37,6 +37,7 @@ final class GitProviderAccountController: ObservableObject { private let openURL: (URL) -> Bool private let multipleAccountAccess: () -> FeatureAccessDecision private let accountAccessPolicy = GitProviderAccountAccessPolicy() + private var cacheOwnerUID: String? private var pendingOAuthSession: GitProviderOAuthSession? private var accountOwnerID: String { store.accountOwnerID } @@ -76,6 +77,12 @@ final class GitProviderAccountController: ObservableObject { func updateMacgitAccount(_ account: AccountSnapshot?) async { let previousAccounts = accounts + // Initial auth restoration (nil -> uid) should retain valid disk cache. + if let previousUID = cacheOwnerUID, previousUID != account?.uid { + accounts = [] + await PullRequestDiskCache.shared.remove() + } + cacheOwnerUID = account?.uid if account == nil { pendingDeviceAuthorization = nil pendingOAuthSession = nil @@ -210,6 +217,8 @@ final class GitProviderAccountController: ObservableObject { errorMessage = nil do { try tokenVault.deleteToken(for: account) + accounts.removeAll { $0.id == account.id } + await PullRequestDiskCache.shared.remove(accountID: account.id) try await sshKeyStore.deleteKey(for: account) try await store.delete(accountID: account.id) accounts.removeAll { $0.id == account.id } diff --git a/macgit/App/PullRequestController.swift b/macgit/App/PullRequestController.swift index 055ad6a..95498ff 100644 --- a/macgit/App/PullRequestController.swift +++ b/macgit/App/PullRequestController.swift @@ -25,18 +25,24 @@ private struct PullRequestListCacheKey: Hashable { let filter: PullRequestListFilter let page: Int let perPage: Int + var diskKey: String { repository.diskKey + "." + ([accountID, "list"] + [filter.rawValue, String(page), String(perPage)]).map { Data($0.utf8).base64EncodedString() }.joined(separator: ".") } + } private struct PullRequestDetailCacheKey: Hashable { let repository: GitRepositoryIdentityKey let accountID: String let number: Int + var diskKey: String { repository.diskKey + "." + ([accountID, "detail"] + [String(number)]).map { Data($0.utf8).base64EncodedString() }.joined(separator: ".") } + } private struct PullRequestChangesCacheKey: Hashable { let repository: GitRepositoryIdentityKey let accountID: String let number: Int + var diskKey: String { repository.diskKey + "." + ([accountID, "changes"] + [String(number)]).map { Data($0.utf8).base64EncodedString() }.joined(separator: ".") } + } private struct GitRepositoryIdentityKey: Hashable { @@ -45,29 +51,18 @@ private struct GitRepositoryIdentityKey: Hashable { let owner: String let name: String + var diskKey: String { + [provider.rawValue, host, owner, name].map { Data($0.utf8).base64EncodedString() }.joined(separator: ".") + } + init(_ repository: GitRepositoryIdentity) { provider = repository.provider - host = repository.hostURL.absoluteString.lowercased() - owner = repository.owner.lowercased() - name = repository.name.lowercased() + host = repository.hostURL.absoluteString + owner = repository.owner + name = repository.name } } -private struct CachedPullRequestListPage { - let value: PullRequestListPage - let expiresAt: Date -} - -private struct CachedPullRequestDetail { - let value: PullRequestDetail - let expiresAt: Date -} - -private struct CachedPullRequestChanges { - let value: [PullRequestChangedFile] - let expiresAt: Date -} - @MainActor final class PullRequestController: ObservableObject { @Published private(set) var items: [PullRequestSummary] = [] @@ -119,9 +114,11 @@ final class PullRequestController: ObservableObject { private let changesCacheTTL: TimeInterval = 300 private let commentRefreshAttempts = 3 private let commentRefreshDelayNanoseconds: UInt64 = 300_000_000 - private var listCache: [PullRequestListCacheKey: CachedPullRequestListPage] = [:] - private var detailCache: [PullRequestDetailCacheKey: CachedPullRequestDetail] = [:] - private var changesCache: [PullRequestChangesCacheKey: CachedPullRequestChanges] = [:] + private let diskCache: PullRequestDiskCache + private var cacheMaintenance: Task? + private var accountObservation: AnyCancellable? + private var listLoadID = UUID() + private var detailLoadID = UUID() private var changesLoadID = UUID() private var createDraftChangesLoadID = UUID() private var createDraftParticipantsLoadID = UUID() @@ -196,7 +193,8 @@ final class PullRequestController: ObservableObject { repositoryURL: repositoryURL ) }, - openURL: @escaping (URL) -> Bool = { _ in false } + openURL: @escaping (URL) -> Bool = { _ in false }, + diskCache: PullRequestDiskCache? = nil ) { self.providerAccountController = providerAccountController self.tokenVault = tokenVault @@ -211,6 +209,25 @@ final class PullRequestController: ObservableObject { self.fetchPullRequestRef = fetchPullRequestRef self.checkoutBranch = checkoutBranch self.openURL = openURL + self.diskCache = diskCache ?? (FirebaseBootstrap.isRunningUnitTests + ? PullRequestDiskCache(url: FileManager.default.temporaryDirectory.appending(path: "PRCacheTests-\(UUID().uuidString).sqlite")) + : .shared) + var previousAccounts = providerAccountController.accounts + accountObservation = providerAccountController.$accounts.dropFirst().sink { [weak self] accounts in + guard let self else { return } + let removed = Set(previousAccounts.map(\.id)).subtracting(accounts.map(\.id)) + let selected = selectedProviderAccountID + let selectedChanged = selected != nil && previousAccounts.first(where: { $0.id == selected }) + != accounts.first(where: { $0.id == selected }) + previousAccounts = accounts + for id in removed { scheduleCacheRemoval(accountID: id) } + if selectedChanged { + releaseVisibleData() + activeToken = nil + activeRepository = nil + selectedProviderAccountID = nil + } + } } var visibleItems: [PullRequestSummary] { @@ -238,9 +255,12 @@ final class PullRequestController: ObservableObject { } func loadPullRequests(repositoryURL: URL, page: Int = 1, forceRefresh: Bool = false) async { + let contextID = UUID() + listLoadID = contextID activeRepositoryURL = repositoryURL guard let remoteName = await remoteNameProvider(repositoryURL), let remoteURLString = await remoteURLProvider(repositoryURL, remoteName) else { + guard listLoadID == contextID, !Task.isCancelled else { return } items = [] resetPagination() activeRemoteName = nil @@ -249,6 +269,7 @@ final class PullRequestController: ObservableObject { errorMessage = "No remotes configured." return } + guard listLoadID == contextID, !Task.isCancelled else { return } activeRemoteName = remoteName await loadPullRequests(remoteURLString: remoteURLString, page: page, forceRefresh: forceRefresh) } @@ -259,8 +280,11 @@ final class PullRequestController: ObservableObject { page: Int = 1, forceRefresh: Bool = false ) async { + let contextID = UUID() + listLoadID = contextID activeRepositoryURL = repositoryURL guard let remoteURLString = await remoteURLProvider(repositoryURL, remoteName) else { + guard listLoadID == contextID, !Task.isCancelled else { return } items = [] resetPagination() activeRemoteName = nil @@ -269,15 +293,22 @@ final class PullRequestController: ObservableObject { errorMessage = "No remotes configured." return } + guard listLoadID == contextID, !Task.isCancelled else { return } activeRemoteName = remoteName await loadPullRequests(remoteURLString: remoteURLString, page: page, forceRefresh: forceRefresh) } func loadPullRequests(remoteURLString: String, page: Int = 1, forceRefresh: Bool = false) async { + let loadID = UUID() + listLoadID = loadID + await cacheMaintenance?.value + guard listLoadID == loadID, !Task.isCancelled else { return } + let cacheGeneration = await diskCache.generation() + guard listLoadID == loadID, !Task.isCancelled else { return } isLoading = true errorMessage = nil accountConnectionHost = nil - defer { isLoading = false } + defer { if listLoadID == loadID { isLoading = false } } guard let remoteIdentity = GitRemoteIdentityResolver.identity( from: remoteURLString, @@ -323,6 +354,10 @@ final class PullRequestController: ObservableObject { errorMessage = matchingAccounts.contains(where: supportsProviderAPI) ? "Reconnect..." : "Connect Account..." return } + if selectedProviderAccountID != apiCredential.account.id + || activeRepository.map({ GitRepositoryIdentityKey($0) }) != GitRepositoryIdentityKey(repository) { + clearSelectedDetail() + } selectedProviderAccountID = apiCredential.account.id let token = apiCredential.token @@ -346,37 +381,38 @@ final class PullRequestController: ObservableObject { perPage: pullRequestPageSize ) if !forceRefresh, - let cached = listCache[cacheKey] { - if cached.expiresAt > Date() { - apply(cached.value) - accountConnectionHost = nil - errorMessage = nil - return - } - listCache.removeValue(forKey: cacheKey) + let cached = await diskCache.value(PullRequestListPage.self, key: cacheKey.diskKey, generation: cacheGeneration) { + guard listLoadID == loadID, !Task.isCancelled else { return } + apply(cached) + accountConnectionHost = nil + errorMessage = nil + return } + guard listLoadID == loadID, !Task.isCancelled else { return } do { let pageResult = try await service.listPullRequests( repository: repository, token: token, - filter: stateFilter, + filter: cacheKey.filter, page: page, perPage: pullRequestPageSize ) - listCache[cacheKey] = CachedPullRequestListPage( - value: pageResult, - expiresAt: Date().addingTimeInterval(listCacheTTL) - ) + guard listLoadID == loadID, !Task.isCancelled else { return } + await diskCache.save(pageResult, key: cacheKey.diskKey, accountID: cacheKey.accountID, + kind: "list", ttl: listCacheTTL, generation: cacheGeneration) + guard listLoadID == loadID, !Task.isCancelled else { return } apply(pageResult) accountConnectionHost = nil errorMessage = nil } catch let error as PullRequestProviderError { + guard listLoadID == loadID, !Task.isCancelled else { return } items = [] resetPagination() accountConnectionHost = nil errorMessage = error.localizedDescription } catch { + guard listLoadID == loadID, !Task.isCancelled else { return } items = [] resetPagination() accountConnectionHost = nil @@ -399,6 +435,11 @@ final class PullRequestController: ObservableObject { } func loadPullRequestDetail(_ summary: PullRequestSummary, forceRefresh: Bool = false) async { + let loadID = UUID() + detailLoadID = loadID + await cacheMaintenance?.value + let cacheGeneration = await diskCache.generation() + guard detailLoadID == loadID, !Task.isCancelled else { return } guard let repository = activeRepository, let token = activeToken, let service = services[repository.provider] else { @@ -408,7 +449,7 @@ final class PullRequestController: ObservableObject { isLoadingDetail = true detailErrorMessage = nil - defer { isLoadingDetail = false } + defer { if detailLoadID == loadID { isLoadingDetail = false } } let cacheKey = PullRequestDetailCacheKey( repository: GitRepositoryIdentityKey(repository), @@ -416,13 +457,12 @@ final class PullRequestController: ObservableObject { number: summary.number ) if !forceRefresh, - let cached = detailCache[cacheKey] { - if cached.expiresAt > Date() { - selectedDetail = cached.value - return - } - detailCache.removeValue(forKey: cacheKey) + let cached = await diskCache.value(PullRequestDetail.self, key: cacheKey.diskKey, generation: cacheGeneration) { + guard detailLoadID == loadID, !Task.isCancelled else { return } + selectedDetail = cached + return } + guard detailLoadID == loadID, !Task.isCancelled else { return } do { let detail = try await service.pullRequestDetail( @@ -430,17 +470,20 @@ final class PullRequestController: ObservableObject { token: token, number: summary.number ) - detailCache[cacheKey] = CachedPullRequestDetail( - value: detail, - expiresAt: Date().addingTimeInterval(detailCacheTTL) - ) + guard detailLoadID == loadID, !Task.isCancelled else { return } + await diskCache.save(detail, key: cacheKey.diskKey, accountID: cacheKey.accountID, + kind: "detail", number: cacheKey.number, ttl: detailCacheTTL, generation: cacheGeneration) + guard detailLoadID == loadID, !Task.isCancelled else { return } selectedDetail = detail } catch { + guard detailLoadID == loadID, !Task.isCancelled else { return } detailErrorMessage = error.localizedDescription } } func clearSelectedDetail() { + detailLoadID = UUID() + isLoadingDetail = false selectedDetail = nil selectedChanges = [] changesErrorMessage = nil @@ -458,6 +501,9 @@ final class PullRequestController: ObservableObject { let loadID = UUID() changesLoadID = loadID + await cacheMaintenance?.value + let cacheGeneration = await diskCache.generation() + guard changesLoadID == loadID, !Task.isCancelled else { return } isLoadingChanges = true changesErrorMessage = nil defer { @@ -472,15 +518,13 @@ final class PullRequestController: ObservableObject { number: summary.number ) if !forceRefresh, - let cached = changesCache[cacheKey] { - if cached.expiresAt > Date() { - guard changesLoadID == loadID, - selectedDetail?.summary.number == summary.number else { return } - selectedChanges = cached.value - return - } - changesCache.removeValue(forKey: cacheKey) + let cached = await diskCache.value([PullRequestChangedFile].self, key: cacheKey.diskKey, generation: cacheGeneration) { + guard changesLoadID == loadID, !Task.isCancelled, + selectedDetail?.summary.number == summary.number else { return } + selectedChanges = cached + return } + guard changesLoadID == loadID, !Task.isCancelled else { return } do { let changes = try await service.pullRequestChanges( @@ -488,15 +532,14 @@ final class PullRequestController: ObservableObject { token: token, number: summary.number ) - changesCache[cacheKey] = CachedPullRequestChanges( - value: changes, - expiresAt: Date().addingTimeInterval(changesCacheTTL) - ) - guard changesLoadID == loadID, + guard changesLoadID == loadID, !Task.isCancelled else { return } + await diskCache.save(changes, key: cacheKey.diskKey, accountID: cacheKey.accountID, + kind: "changes", number: cacheKey.number, ttl: changesCacheTTL, generation: cacheGeneration) + guard changesLoadID == loadID, !Task.isCancelled, selectedDetail?.summary.number == summary.number else { return } selectedChanges = changes } catch { - guard changesLoadID == loadID, + guard changesLoadID == loadID, !Task.isCancelled, selectedDetail?.summary.number == summary.number else { return } changesErrorMessage = error.localizedDescription } @@ -884,25 +927,43 @@ final class PullRequestController: ObservableObject { } private func invalidateListCache() { - listCache.removeAll() + listLoadID = UUID() + isLoading = false + scheduleCacheRemoval(kind: "list") } private func invalidateDetailCache(for number: Int? = nil) { - guard let number else { - detailCache.removeAll() - return - } - detailCache = detailCache.filter { $0.key.number != number } + detailLoadID = UUID() + isLoadingDetail = false + scheduleCacheRemoval(kind: "detail", number: number) } private func invalidateChangesCache() { - changesCache.removeAll() + changesLoadID = UUID() + isLoadingChanges = false + scheduleCacheRemoval(kind: "changes") + } + + private func scheduleCacheRemoval(accountID: String? = nil, kind: String? = nil, number: Int? = nil) { + let previous = cacheMaintenance + let cache = diskCache + cacheMaintenance = Task { + await previous?.value + await cache.remove(accountID: accountID, kind: kind, number: number) + } } func clearSessionCaches() { - invalidateListCache() - invalidateDetailCache() - invalidateChangesCache() + releaseVisibleData() + scheduleCacheRemoval() + } + + func releaseVisibleData() { + listLoadID = UUID() + isLoading = false + items = [] + resetPagination() + clearSelectedDetail() } private func apiCredential(for accounts: [GitProviderAccount]) -> ( diff --git a/macgit/Models/PullRequestChangedFile.swift b/macgit/Models/PullRequestChangedFile.swift index 9f13725..3215276 100644 --- a/macgit/Models/PullRequestChangedFile.swift +++ b/macgit/Models/PullRequestChangedFile.swift @@ -18,7 +18,7 @@ import Foundation -struct PullRequestChangedFile: Identifiable, Equatable { +nonisolated struct PullRequestChangedFile: Identifiable, Equatable, Codable { var id: String { "\(previousPath ?? "")->\(path)" } let path: String @@ -29,7 +29,7 @@ struct PullRequestChangedFile: Identifiable, Equatable { let patch: String? let patchUnavailableReason: String? - var diffHunks: [DiffHunk] { + @MainActor var diffHunks: [DiffHunk] { guard let patch else { return [] } return DiffParser.parse(patch) } diff --git a/macgit/Models/PullRequestModels.swift b/macgit/Models/PullRequestModels.swift index 2847ddb..94ea17d 100644 --- a/macgit/Models/PullRequestModels.swift +++ b/macgit/Models/PullRequestModels.swift @@ -18,14 +18,14 @@ import Foundation -enum PullRequestState: String, Codable, Equatable { +nonisolated enum PullRequestState: String, Codable, Equatable { case open case closed case merged case draft } -enum PullRequestListFilter: String, CaseIterable, Identifiable { +nonisolated enum PullRequestListFilter: String, CaseIterable, Identifiable, Codable { case open = "Open" case closed = "Closed" case all = "All" @@ -55,7 +55,7 @@ enum PullRequestListFilter: String, CaseIterable, Identifiable { } } -struct PullRequestListPage: Equatable { +nonisolated struct PullRequestListPage: Equatable, Codable { var items: [PullRequestSummary] var page: Int var perPage: Int @@ -77,7 +77,7 @@ struct PullRequestListPage: Equatable { } } -enum PullRequestCheckState: String, Codable, Equatable { +nonisolated enum PullRequestCheckState: String, Codable, Equatable { case unknown case noChecks case pending @@ -86,24 +86,24 @@ enum PullRequestCheckState: String, Codable, Equatable { case error } -enum PullRequestMergeReadiness: String, Codable, Equatable { +nonisolated enum PullRequestMergeReadiness: String, Codable, Equatable { case unknown case ready case blocked } -struct PullRequestAuthor: Equatable, Codable { +nonisolated struct PullRequestAuthor: Equatable, Codable { var username: String var avatarURL: URL? } -struct PullRequestBranchRef: Equatable, Codable { +nonisolated struct PullRequestBranchRef: Equatable, Codable { var label: String var ref: String var sha: String? } -struct PullRequestSummary: Identifiable, Equatable, Codable { +nonisolated struct PullRequestSummary: Identifiable, Equatable, Codable { var id: Int { number } var number: Int var title: String @@ -147,7 +147,7 @@ struct PullRequestSummary: Identifiable, Equatable, Codable { } } -struct PullRequestComment: Identifiable, Equatable, Codable { +nonisolated struct PullRequestComment: Identifiable, Equatable, Codable { var id: Int var author: PullRequestAuthor var body: String @@ -156,7 +156,7 @@ struct PullRequestComment: Identifiable, Equatable, Codable { var updatedAt: Date } -struct PullRequestDetail: Identifiable, Equatable, Codable { +nonisolated struct PullRequestDetail: Identifiable, Equatable, Codable { var id: Int { summary.id } var summary: PullRequestSummary var body: String diff --git a/macgit/Services/Commit.swift b/macgit/Services/Commit.swift index 6444051..6474fa6 100644 --- a/macgit/Services/Commit.swift +++ b/macgit/Services/Commit.swift @@ -55,7 +55,7 @@ nonisolated struct CommitFileChange: Identifiable, Hashable, Sendable { var oldPath: String? = nil } -nonisolated enum CommitFileStatus: String, Sendable { +nonisolated enum CommitFileStatus: String, Sendable, Codable { case added = "A" case modified = "M" case deleted = "D" diff --git a/macgit/Services/PullRequestDiskCache.swift b/macgit/Services/PullRequestDiskCache.swift new file mode 100644 index 0000000..6684f77 --- /dev/null +++ b/macgit/Services/PullRequestDiskCache.swift @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation +import SQLite3 + +/// Disposable, bounded cache. Payloads are queried individually, never mirrored in RAM. +actor PullRequestDiskCache { + static let shared: PullRequestDiskCache = { + if ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil + || NSClassFromString("XCTestCase") != nil { + return PullRequestDiskCache(url: FileManager.default.temporaryDirectory + .appending(path: "PRCacheTestSession-\(UUID().uuidString).sqlite")) + } + return PullRequestDiskCache() + }() + private let url: URL + private let maximumBytes: Int + private let maximumEntryBytes: Int + private var epoch = UUID() + + init(url: URL = URL.cachesDirectory.appending(path: "dev.thanhtran.macgit/pull-requests-v1.sqlite"), + maximumBytes: Int = 32 * 1024 * 1024, maximumEntryBytes: Int = 2 * 1024 * 1024) { + self.url = url + self.maximumBytes = maximumBytes + self.maximumEntryBytes = maximumEntryBytes + } + + func generation() -> UUID { epoch } + + func value(_ type: T.Type, key: String, generation: UUID, + now: Date = .now) -> T? { + guard generation == epoch else { return nil } + return try? database { db in + var result: T? + try query(db, "SELECT payload FROM entries WHERE key = ? AND expires > ?", [key, String(now.timeIntervalSince1970)]) { statement in + let count = Int(sqlite3_column_bytes(statement, 0)) + guard count <= maximumEntryBytes, let bytes = sqlite3_column_blob(statement, 0) else { return } + result = try? JSONDecoder().decode(type, from: Data(bytes: bytes, count: count)) + } + if result != nil { + try query(db, "UPDATE entries SET accessed = ? WHERE key = ?", [String(now.timeIntervalSince1970), key]) + } else { + try query(db, "DELETE FROM entries WHERE key = ?", [key]) + } + return result + } + } + + func save(_ value: T, key: String, accountID: String, + kind: String, number: Int = 0, ttl: TimeInterval, + generation: UUID, now: Date = .now) { + guard generation == epoch else { return } + guard let data = try? JSONEncoder().encode(value), + data.count <= min(maximumEntryBytes, maximumBytes) else { + try? database { db in try query(db, "DELETE FROM entries WHERE key = ?", [key]) } + return + } + try? database { db in + try query(db, "BEGIN IMMEDIATE") + do { + try query(db, "DELETE FROM entries WHERE expires <= ?", [String(now.timeIntervalSince1970)]) + try query(db, "INSERT OR REPLACE INTO entries (key, account, kind, number, expires, accessed, payload) VALUES (?, ?, ?, ?, ?, ?, ?)", + [key, accountID, kind, String(number), String(now.addingTimeInterval(ttl).timeIntervalSince1970), + String(now.timeIntervalSince1970), String(decoding: data, as: UTF8.self)]) + var size = 0 + var count = 0 + try query(db, "SELECT COALESCE(SUM(length(CAST(payload AS BLOB))), 0), COUNT(*) FROM entries") { + size = Int(sqlite3_column_int64($0, 0)); count = Int(sqlite3_column_int64($0, 1)) + } + while size > maximumBytes || count > 300 { + var oldestKey: String? + var bytes = 0 + try query(db, "SELECT key, length(CAST(payload AS BLOB)) FROM entries ORDER BY accessed, key LIMIT 1") { + oldestKey = String(cString: sqlite3_column_text($0, 0)) + bytes = Int(sqlite3_column_int64($0, 1)) + } + guard let oldestKey else { break } + try query(db, "DELETE FROM entries WHERE key = ?", [oldestKey]) + size -= bytes; count -= 1 + } + try query(db, "COMMIT") + } catch { + try? query(db, "ROLLBACK") + throw error + } + } + } + + /// Bumping the epoch also prevents already-running network requests from repopulating removed data. + func remove(accountID: String? = nil, kind: String? = nil, number: Int? = nil) { + epoch = UUID() + do { + try database { db in + var clauses: [String] = [] + var values: [String] = [] + if let accountID { clauses.append("account = ?"); values.append(accountID) } + if let kind { clauses.append("kind = ?"); values.append(kind) } + if let number { clauses.append("number = ?"); values.append(String(number)) } + try query(db, "DELETE FROM entries" + (clauses.isEmpty ? "" : " WHERE " + clauses.joined(separator: " AND ")), values) + } + } catch { + // A corrupt disposable cache must still be removable on logout/Clear Cache. + try? FileManager.default.removeItem(at: url) + try? FileManager.default.removeItem(atPath: url.path + "-journal") + } + } + + private func database(_ operation: (OpaquePointer) throws -> T) throws -> T { + try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) + var connection: OpaquePointer? + guard sqlite3_open(url.path, &connection) == SQLITE_OK, let db = connection else { + if let connection { sqlite3_close(connection) } + throw CocoaError(.fileReadUnknown) + } + defer { sqlite3_close(db) } + try FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) + sqlite3_busy_timeout(db, 500) + try query(db, "PRAGMA auto_vacuum = FULL") + try query(db, "PRAGMA max_page_count = 10240") + var version = 0 + try query(db, "PRAGMA user_version") { version = Int(sqlite3_column_int($0, 0)) } + guard version <= 1 else { throw CocoaError(.fileReadCorruptFile) } + try query(db, "CREATE TABLE IF NOT EXISTS entries (key TEXT PRIMARY KEY, account TEXT NOT NULL, kind TEXT NOT NULL, number INTEGER NOT NULL, expires REAL NOT NULL, accessed REAL NOT NULL, payload TEXT NOT NULL)") + if version == 0 { try query(db, "PRAGMA user_version = 1") } + return try operation(db) + } + + private func query(_ db: OpaquePointer, _ sql: String, _ values: [String] = [], + row: (OpaquePointer) throws -> Void = { _ in }) throws { + var prepared: OpaquePointer? + guard sqlite3_prepare_v2(db, sql, -1, &prepared, nil) == SQLITE_OK, let statement = prepared else { + throw CocoaError(.fileReadCorruptFile) + } + defer { sqlite3_finalize(statement) } + for (index, value) in values.enumerated() { + let status = value.withCString { + sqlite3_bind_text(statement, Int32(index + 1), $0, -1, unsafeBitCast(-1, to: sqlite3_destructor_type.self)) + } + guard status == SQLITE_OK else { throw CocoaError(.fileWriteUnknown) } + } + var status = sqlite3_step(statement) + while status == SQLITE_ROW { try row(statement); status = sqlite3_step(statement) } + guard status == SQLITE_DONE else { throw CocoaError(.fileWriteUnknown) } + } +} diff --git a/macgit/Views/Common/AdvancedSettingsView.swift b/macgit/Views/Common/AdvancedSettingsView.swift index 1774f03..a47f6b3 100644 --- a/macgit/Views/Common/AdvancedSettingsView.swift +++ b/macgit/Views/Common/AdvancedSettingsView.swift @@ -234,6 +234,7 @@ struct AdvancedSettingsView: View { clearMessages() Task { await GitStatusService.shared.clearSessionCaches() + await PullRequestDiskCache.shared.remove() await MainActor.run { NotificationCenter.default.post( name: .advancedClearSessionCaches, diff --git a/macgit/Views/PullRequests/PullRequestListView.swift b/macgit/Views/PullRequests/PullRequestListView.swift index 43eac09..0d4648b 100644 --- a/macgit/Views/PullRequests/PullRequestListView.swift +++ b/macgit/Views/PullRequests/PullRequestListView.swift @@ -155,6 +155,14 @@ struct PullRequestListView: View { } message: { Text(controller.detailErrorMessage ?? "Could not load pull request details.") } + .onDisappear { controller.releaseVisibleData() } + .onReceive(NotificationCenter.default.publisher(for: .advancedClearSessionCaches)) { _ in + closeDetail() + Task { + guard await authorizeAction() else { return } + await controller.loadPullRequests(repositoryURL: repositoryURL, forceRefresh: true) + } + } .task(id: repositoryURL) { guard await authorizeAction() else { return } closeDetail() diff --git a/macgitTests/PullRequestControllerTests.swift b/macgitTests/PullRequestControllerTests.swift index 621f339..9cb1187 100644 --- a/macgitTests/PullRequestControllerTests.swift +++ b/macgitTests/PullRequestControllerTests.swift @@ -343,7 +343,7 @@ final class PullRequestControllerTests: XCTestCase { XCTAssertEqual(service.receivedDetailNumber, 12) } - func testLoadPullRequestsUsesMemoryCacheUntilForcedRefresh() async throws { + func testLoadPullRequestsUsesDiskCacheUntilForcedRefresh() async throws { let account = makeAccount() let token = makeToken() let service = FakePullRequestProvider(result: .success([makeSummary()])) @@ -373,7 +373,7 @@ final class PullRequestControllerTests: XCTestCase { XCTAssertEqual(service.listCallCount, 2) } - func testLoadPullRequestDetailUsesMemoryCache() async throws { + func testLoadPullRequestDetailUsesDiskCache() async throws { let account = makeAccount() let token = makeToken() let detail = PullRequestDetail( @@ -1089,6 +1089,82 @@ final class PullRequestControllerTests: XCTestCase { XCTAssertEqual(service.receivedDetailNumber, summary.number) } + func testDiskCacheSurvivesNewControllerAndSeparatesRepositoryFilterAndPage() async throws { + let folder = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? FileManager.default.removeItem(at: folder) } + let url = folder.appending(path: "cache.sqlite") + let account = makeAccount() + let vault = FakePullRequestTokenVault(tokensByAccountID: [account.id: makeToken()]) + let accounts = GitProviderAccountController(store: FakePullRequestAccountStore(accounts: [account]), tokenVault: vault) + await accounts.reload() + let service = FakePullRequestProvider(result: .success([makeSummary()])) + let first = PullRequestController(providerAccountController: accounts, tokenVault: vault, + services: [.github: service], diskCache: PullRequestDiskCache(url: url)) + let remote = "https://github.com/octocat/Hello-World.git" + await first.loadPullRequests(remoteURLString: remote) + first.releaseVisibleData() + XCTAssertTrue(first.items.isEmpty) + let reopened = PullRequestController(providerAccountController: accounts, tokenVault: vault, + services: [.github: service], diskCache: PullRequestDiskCache(url: url)) + await reopened.loadPullRequests(remoteURLString: remote) + XCTAssertEqual(service.listCallCount, 1) + XCTAssertEqual(reopened.items.count, 1) + await reopened.loadPullRequests(remoteURLString: remote, page: 2) + reopened.stateFilter = .closed + await reopened.loadPullRequests(remoteURLString: remote) + await reopened.loadPullRequests(remoteURLString: "https://github.com/octocat/Another.git") + XCTAssertEqual(service.listCallCount, 4) + } + + func testDiskCacheSeparatesAccountsForSameRepository() async throws { + let folder = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? FileManager.default.removeItem(at: folder) } + let cache = PullRequestDiskCache(url: folder.appending(path: "cache.sqlite")) + let service = FakePullRequestProvider(result: .success([makeSummary()])) + for id in ["alice", "bob"] { + let account = makeAccount(id: id) + let vault = FakePullRequestTokenVault(tokensByAccountID: [account.id: makeToken()]) + let accounts = GitProviderAccountController(store: FakePullRequestAccountStore(accounts: [account]), tokenVault: vault) + await accounts.reload() + let controller = PullRequestController(providerAccountController: accounts, tokenVault: vault, + services: [.github: service], diskCache: cache) + await controller.loadPullRequests(remoteURLString: "https://github.com/octocat/Hello-World.git") + } + XCTAssertEqual(service.listCallCount, 2) + } + + func testClearingCacheDiscardsSuspendedListResponse() async throws { + let folder = FileManager.default.temporaryDirectory.appending(path: UUID().uuidString) + defer { try? FileManager.default.removeItem(at: folder) } + let cache = PullRequestDiskCache(url: folder.appending(path: "cache.sqlite")) + let account = makeAccount() + let vault = FakePullRequestTokenVault(tokensByAccountID: [account.id: makeToken()]) + let accounts = GitProviderAccountController(store: FakePullRequestAccountStore(accounts: [account]), tokenVault: vault) + await accounts.reload() + let service = FakePullRequestProvider(result: .success([makeSummary()])) + let started = expectation(description: "Network list pending") + var continuation: CheckedContinuation? + service.beforeListResponse = { + await withCheckedContinuation { + continuation = $0 + started.fulfill() + } + } + let controller = PullRequestController(providerAccountController: accounts, tokenVault: vault, + services: [.github: service], diskCache: cache) + let remote = "https://github.com/octocat/Hello-World.git" + let old = Task { await controller.loadPullRequests(remoteURLString: remote) } + await fulfillment(of: [started], timeout: 2) + controller.clearSessionCaches() + service.beforeListResponse = nil + continuation?.resume() + await old.value + XCTAssertTrue(controller.items.isEmpty) + await controller.loadPullRequests(remoteURLString: remote) + XCTAssertEqual(service.listCallCount, 2) + XCTAssertEqual(controller.items.count, 1) + } + private func makeAccount( id: String = "macgit-user-1:github:github.com:583231", scopes: [String] = ["repo", "read:user"], @@ -1204,6 +1280,7 @@ private final class FakePullRequestProvider: PullRequestProviding { private(set) var receivedPerPage: Int? private(set) var receivedDetailNumber: Int? private(set) var receivedChangesNumber: Int? + var beforeListResponse: (() async -> Void)? private(set) var listCallCount = 0 private(set) var detailCallCount = 0 private(set) var changesCallCount = 0 @@ -1244,6 +1321,7 @@ private final class FakePullRequestProvider: PullRequestProviding { perPage: Int ) async throws -> PullRequestListPage { listCallCount += 1 + await beforeListResponse?() receivedRepository = repository receivedToken = token receivedFilter = filter diff --git a/macgitTests/PullRequestDiskCacheTests.swift b/macgitTests/PullRequestDiskCacheTests.swift new file mode 100644 index 0000000..015cf97 --- /dev/null +++ b/macgitTests/PullRequestDiskCacheTests.swift @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +final class PullRequestDiskCacheTests: XCTestCase { + private func location() -> URL { + FileManager.default.temporaryDirectory.appending(path: "PRDiskCache-\(UUID().uuidString)/cache.sqlite") + } + + func testPersistenceAcrossInstancesAndTTL() async throws { + let url = location() + defer { try? FileManager.default.removeItem(at: url.deletingLastPathComponent()) } + let first = PullRequestDiskCache(url: url) + let now = Date.now + let epoch = await first.generation() + await first.save(["one", "two"], key: "list", accountID: "alice", kind: "list", ttl: 60, generation: epoch, now: now) + let second = PullRequestDiskCache(url: url) + let nextEpoch = await second.generation() + let cached = await second.value([String].self, key: "list", generation: nextEpoch, now: now.addingTimeInterval(59)) + XCTAssertEqual(cached, ["one", "two"]) + let expired = await second.value([String].self, key: "list", generation: nextEpoch, now: now.addingTimeInterval(60)) + XCTAssertNil(expired) + } + + func testAccountRemovalRejectsLateWriteAndPreservesOtherAccount() async { + let url = location() + defer { try? FileManager.default.removeItem(at: url.deletingLastPathComponent()) } + let cache = PullRequestDiskCache(url: url) + let epoch = await cache.generation() + await cache.save("alice", key: "a", accountID: "alice", kind: "detail", ttl: 60, generation: epoch) + await cache.save("bob", key: "b", accountID: "bob", kind: "detail", ttl: 60, generation: epoch) + await cache.remove(accountID: "alice") + await cache.save("late alice response", key: "a", accountID: "alice", kind: "detail", ttl: 60, generation: epoch) + let current = await cache.generation() + let alice = await cache.value(String.self, key: "a", generation: current) + let bob = await cache.value(String.self, key: "b", generation: current) + XCTAssertNil(alice) + XCTAssertEqual(bob, "bob") + } + + func testEvictsOldestPayloadAndSkipsOversizedReplacement() async { + let url = location() + defer { try? FileManager.default.removeItem(at: url.deletingLastPathComponent()) } + let cache = PullRequestDiskCache(url: url, maximumBytes: 100, maximumEntryBytes: 80) + let epoch = await cache.generation() + let now = Date.now + let payload = String(repeating: "x", count: 60) + await cache.save(payload, key: "old", accountID: "a", kind: "list", ttl: 60, generation: epoch, now: now) + await cache.save(payload, key: "new", accountID: "a", kind: "list", ttl: 60, generation: epoch, now: now.addingTimeInterval(1)) + let old = await cache.value(String.self, key: "old", generation: epoch, now: now.addingTimeInterval(2)) + let new = await cache.value(String.self, key: "new", generation: epoch, now: now.addingTimeInterval(2)) + XCTAssertNil(old) + XCTAssertEqual(new, payload) + await cache.save(String(repeating: "z", count: 100), key: "new", accountID: "a", kind: "list", ttl: 60, generation: epoch) + let oversized = await cache.value(String.self, key: "new", generation: epoch) + XCTAssertNil(oversized) + } + + func testCorruptionFallsBackToMissAndClearRepairsCache() async throws { + let url = location() + defer { try? FileManager.default.removeItem(at: url.deletingLastPathComponent()) } + try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("invalid SQLite".utf8).write(to: url) + let cache = PullRequestDiskCache(url: url) + let epoch = await cache.generation() + let value = await cache.value(String.self, key: "key", generation: epoch) + XCTAssertNil(value) + await cache.remove() + let current = await cache.generation() + await cache.save("recovered", key: "key", accountID: "a", kind: "list", ttl: 60, generation: current) + let recovered = await cache.value(String.self, key: "key", generation: current) + XCTAssertEqual(recovered, "recovered") + } + + func testInvalidationTargetsKindAndPRNumber() async { + let url = location() + defer { try? FileManager.default.removeItem(at: url.deletingLastPathComponent()) } + let cache = PullRequestDiskCache(url: url) + let epoch = await cache.generation() + await cache.save("list", key: "list", accountID: "a", kind: "list", ttl: 60, generation: epoch) + await cache.save("one", key: "one", accountID: "a", kind: "detail", number: 1, ttl: 60, generation: epoch) + await cache.save("two", key: "two", accountID: "a", kind: "detail", number: 2, ttl: 60, generation: epoch) + await cache.remove(kind: "detail", number: 1) + let current = await cache.generation() + let list = await cache.value(String.self, key: "list", generation: current) + let one = await cache.value(String.self, key: "one", generation: current) + let two = await cache.value(String.self, key: "two", generation: current) + XCTAssertEqual(list, "list") + XCTAssertNil(one) + XCTAssertEqual(two, "two") + } +} From 33166a1d21e510d0c7f36b1529be4c695e5de57b Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Sun, 27 Sep 2026 22:41:23 +0700 Subject: [PATCH 04/10] perf: load local data on demand --- .../plans/2026-09-27-memory-cold-start.md | 32 ++++-- .../GitFlowConfigurationSyncController.swift | 12 +-- macgit/App/RepositoryBookmarkController.swift | 69 +++++++------ .../RepositoryCommitRuleSyncController.swift | 28 +++--- .../App/RepositoryVisibilityController.swift | 2 +- .../GitProviderAccountPreferenceStore.swift | 4 +- macgit/Services/LocalDataError.swift | 2 + macgit/Services/LocalDataStore.swift | 36 ++++++- macgit/Services/LocalDataTransaction.swift | 6 +- .../LocalGitProviderAccountStore.swift | 30 +++--- macgit/Services/LocalSQLiteDatabase.swift | 38 +++++-- macgit/Services/RepoSettingsStore.swift | 4 +- .../Services/RepositoryVisibilityCache.swift | 8 +- macgit/Views/MainWindow/MainWindowView.swift | 2 +- .../GitFlowConfigurationSyncTests.swift | 3 +- macgitTests/LocalDataMigrationTests.swift | 99 ++++++++++++------- macgitTests/LocalDataOnDemandTests.swift | 71 +++++++++++++ macgitTests/RepoSettingsStoreTests.swift | 4 +- macgitTests/RepositoryBookmarkTests.swift | 27 ++--- ...ositoryCommitRuleSyncControllerTests.swift | 8 +- .../RepositoryVisibilityControllerTests.swift | 19 ++-- 21 files changed, 345 insertions(+), 159 deletions(-) create mode 100644 macgitTests/LocalDataOnDemandTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index 97a49c7..42dd4ad 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -1,6 +1,6 @@ # Cải thiện RAM và cold start -Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown) và phần 3 (PR cache). Phần 4–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown) phần 3 (PR cache) và phần 4 (local storage). Phần 5–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. @@ -53,12 +53,12 @@ Kiểm tra: phân trang/filter, mở lại app, TTL, offline, cache hỏng, gi Điểm sửa chính: `LocalDataStore`, `LocalSQLiteDatabase`, `LocalDataTransaction` và các store sử dụng chúng. -- [ ] Liệt kê collection và consumer; xác định dữ liệu thật sự cần trước khi hiện màn hình đầu tiên. -- [ ] Thay đọc toàn bộ records bằng truy vấn theo collection/entity cần dùng. -- [ ] Chuyển các consumer sang tải bất đồng bộ hoặc snapshot có phạm vi rõ ràng; không đưa SQLite I/O vào main thread. -- [ ] Giữ serialization của writer và tính atomic của transaction nhiều collection. -- [ ] Giữ migration, verification, retry và thông báo lỗi; tránh mất dữ liệu khi import bị gián đoạn. -- [ ] Giải phóng snapshot không cần; tránh giải mã lại cùng dữ liệu trong mỗi lần render. +- [x] Liệt kê collection và consumer; xác định dữ liệu thật sự cần trước khi hiện màn hình đầu tiên. +- [x] Thay đọc toàn bộ records bằng truy vấn theo collection/entity cần dùng. +- [x] Chuyển các consumer sang tải bất đồng bộ hoặc snapshot có phạm vi rõ ràng; không đưa SQLite I/O vào main thread. +- [x] Giữ serialization của writer và tính atomic của transaction nhiều collection. +- [x] Giữ migration, verification, retry và thông báo lỗi; tránh mất dữ liệu khi import bị gián đoạn. +- [x] Giải phóng snapshot không cần; tránh giải mã lại cùng dữ liệu trong mỗi lần render. Kiểm tra: DB mới/cũ, migration lỗi giữa chừng, ghi đồng thời, transaction lỗi, dữ liệu nhiều collection và tải lại sau ghi. Đây là phần thay đổi contract, cần review riêng. @@ -133,3 +133,21 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Gỡ provider account xóa cache tương ứng. Logout/đổi tài khoản Commit+ xóa cache phiên trước; khôi phục auth ban đầu không xóa cache hợp lệ. Tests dùng DB tạm riêng. - Đã kiểm tra persistence qua cache/controller mới, TTL, giới hạn/oversize, corruption, namespace account/repo/filter/page và response đến muộn sau Clear Cache. - Validation cuối trên mã bàn giao: **BUILD SUCCEEDED**, **85/85 test PASS**, `git diff --check` sạch. Không đo baseline, không tự mở lại app để kiểm tra UI. Chưa commit phần 3. + +### Phần 4 — Local storage (2026-09-27) + +- `prepare()` chỉ giữ ba collection nhỏ phục vụ API định tuyến credential đồng bộ; bỏ snapshot RAM của toàn bộ database. + +| Collection | Consumer và thời điểm đọc | +| --- | --- | +| `providerAccounts`, `providerPreferences`, `sshPaths` | Account/credential routing; snapshot resident sau prepare | +| `repoSettings` | RepoSettingsStore và commit-rule sync; đọc theo repository khi cần | +| `bookmarks`, `bookmarkPaths`, `bookmarkUploads`, `bookmarkDeletes` | Bookmark controller; snapshot có phạm vi khi load/sync, giữ model UI cần hiển thị | +| `providerDeletions`, `providerSyncedIdentities` | Provider account sync; đọc khi reconcile | +| `repositoryVisibility` | Visibility controller; đọc theo repository | +| `gitFlowPending`, `commitRulePending` | Sync controller; đọc marker khi đồng bộ | + +- SQLite I/O vẫn chạy trên actor database. Snapshot transaction chỉ chứa collection được khai báo, được giải phóng sau thao tác; writer vẫn serialize và commit nhiều collection atomically. Resident snapshot chỉ cập nhật sau commit thành công. +- Migration vẫn import trong transaction và kiểm tra từng record trước khi đánh dấu hoàn tất; không nạp toàn bộ database để verification. Lỗi đọc pending marker được chuyển tới xử lý lỗi sync để tránh hiểu nhầm là không có thay đổi local. +- Thêm coverage cho đọc có phạm vi, không giữ collection không liên quan, rollback khi đọc thiếu scope, ghi đồng thời và cập nhật resident snapshot. +- Validation: build macOS **PASS**, **60/60 test PASS** trong các nhóm local storage, migration, bookmark, settings, sync, visibility và credential stores; commit-rule sync sau thay đổi cuối **4/4 test PASS**. Không launch/relaunch app; chưa đo mức giảm RAM/cold start. diff --git a/macgit/App/GitFlowConfigurationSyncController.swift b/macgit/App/GitFlowConfigurationSyncController.swift index b2d80c4..c7fcb93 100644 --- a/macgit/App/GitFlowConfigurationSyncController.swift +++ b/macgit/App/GitFlowConfigurationSyncController.swift @@ -98,7 +98,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { let uploadID = pendingUploadID(uid: uid, repositoryID: identity.documentID) do { - if let pendingVersion = pendingVersion(uploadID) { + if let pendingVersion = try await pendingVersion(uploadID) { if case .value(let localConfiguration) = localResult { try await upload( localConfiguration, @@ -117,7 +117,7 @@ final class GitFlowConfigurationSyncController: ObservableObject { uid: uid ) { let latestLocalResult = await localStore.loadResult(in: repositoryURL) - let pendingVersion = pendingVersion(uploadID) + let pendingVersion = try await pendingVersion(uploadID) if pendingVersion != nil || localConfigurationChanged( from: localResult, to: latestLocalResult @@ -233,20 +233,20 @@ final class GitFlowConfigurationSyncController: ObservableObject { "\(uid)|\(repositoryID)" } - private func pendingVersion(_ id: String) -> String? { - try? dataStore.value(String.self, in: "gitFlowPending", id: id) + private func pendingVersion(_ id: String) async throws -> String? { + try await dataStore.readValue(String.self, in: "gitFlowPending", id: id) } private func markPendingUpload(_ id: String) async throws -> String { let version = UUID().uuidString - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["gitFlowPending"]) { transaction in try transaction.set(version, in: "gitFlowPending", id: id) } return version } private func clearPendingUpload(_ id: String, version: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["gitFlowPending"]) { transaction in guard try transaction.value(String.self, in: "gitFlowPending", id: id) == version else { return } transaction.remove(in: "gitFlowPending", id: id) } diff --git a/macgit/App/RepositoryBookmarkController.swift b/macgit/App/RepositoryBookmarkController.swift index d690bfa..bdfbcaf 100644 --- a/macgit/App/RepositoryBookmarkController.swift +++ b/macgit/App/RepositoryBookmarkController.swift @@ -55,6 +55,8 @@ final class RepositoryBookmarkController: ObservableObject { private let cloudStore: RepositoryBookmarkCloudStore? private let dataStore: LocalDataStore @Published private var activeUID: String? + private var loadID = UUID() + private var accountUpdateID = UUID() private var observation: ObservationToken? init(cloudStore: RepositoryBookmarkCloudStore?, dataStore: LocalDataStore? = nil) { @@ -64,14 +66,20 @@ final class RepositoryBookmarkController: ObservableObject { deinit { observation?.cancel() } - func load() throws { - bookmarks = try dataStore.values(RepositoryBookmark.self, in: "bookmarks").values.sorted { + func load() async throws { + let request = UUID() + loadID = request + let snapshot = try await dataStore.snapshot(collections: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) + guard request == loadID else { return } + let loadedBookmarks = try snapshot.values(RepositoryBookmark.self, in: "bookmarks").values.sorted { $0.name.localizedCaseInsensitiveCompare($1.name) == .orderedAscending } - localPaths = try dataStore.values(String.self, in: "bookmarkPaths") - mismatchedBookmarkIDs.formIntersection(Set(bookmarks.map(\.id))) - let uploads = try dataStore.values(String.self, in: "bookmarkUploads") - let deletes = try dataStore.values(String.self, in: "bookmarkDeletes") + let loadedPaths = try snapshot.values(String.self, in: "bookmarkPaths") + let uploads = try snapshot.values(String.self, in: "bookmarkUploads") + let deletes = try snapshot.values(String.self, in: "bookmarkDeletes") + bookmarks = loadedBookmarks + localPaths = loadedPaths + mismatchedBookmarkIDs.formIntersection(Set(loadedBookmarks.map(\.id))) hasPendingChanges = !uploads.isEmpty || !deletes.isEmpty } @@ -82,13 +90,16 @@ final class RepositoryBookmarkController: ObservableObject { isRetryingSync = true defer { isRetryingSync = false } await flushPendingChanges(uid: uid, cloudStore: cloudStore) - do { try load() } catch { errorMessage = error.localizedDescription } + do { try await load() } catch { errorMessage = error.localizedDescription } } func updateAccount(_ account: AccountSnapshot?) async { + let request = UUID() + accountUpdateID = request do { try await dataStore.prepare() - try load() + try await load() + guard request == accountUpdateID else { return } let uid = account?.uid guard uid != activeUID else { return } observation?.cancel() @@ -138,7 +149,7 @@ final class RepositoryBookmarkController: ObservableObject { ) async throws -> RepositoryBookmark { let currentRemotes = try await GitStatusService.shared.repositoryBookmarkRemotes(in: repositoryURL) guard currentRemotes.contains(remote) else { throw RepositoryBookmarkError.remoteChanged } - let result = try await dataStore.transaction { transaction in + let result = try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in guard let current = try transaction.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id), current.canonicalKey == bookmark.canonicalKey else { throw RepositoryBookmarkError.bookmarkChanged } let identity = remote.identity @@ -170,12 +181,12 @@ final class RepositoryBookmarkController: ObservableObject { } mismatchedBookmarkIDs.remove(bookmark.id) mismatchedBookmarkIDs.remove(result.id) - try load() + try await load() if let uid = activeUID, let cloudStore { await upload(result, uid: uid, cloudStore: cloudStore) // Keep the cloud's old bookmark until its replacement has been saved. if result.id != bookmark.id, - try dataStore.value(String.self, in: "bookmarkUploads", id: result.id) == nil { + try await dataStore.readValue(String.self, in: "bookmarkUploads", id: result.id) == nil { await deleteFromCloud(bookmark.id, uid: uid, cloudStore: cloudStore) } } @@ -187,7 +198,7 @@ final class RepositoryBookmarkController: ObservableObject { guard let identity = RepositoryBookmarkIdentity.resolve(remoteURLString: remoteURLString) else { throw RepositoryBookmarkError.unsupportedRemote } - let bookmark = try await dataStore.transaction { transaction in + let bookmark = try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in let existing = try transaction.values(RepositoryBookmark.self, in: "bookmarks").values.first { $0.canonicalKey == identity.canonicalKey } let bookmark = existing ?? RepositoryBookmark(identity: identity) try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) @@ -198,29 +209,29 @@ final class RepositoryBookmarkController: ObservableObject { } return bookmark } - try load() + try await load() if let uid = activeUID, let cloudStore { await upload(bookmark, uid: uid, cloudStore: cloudStore) } return bookmark } func removeBookmark(_ bookmark: RepositoryBookmark) async { do { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in transaction.remove(in: "bookmarks", id: bookmark.id) transaction.remove(in: "bookmarkPaths", id: bookmark.id) transaction.remove(in: "bookmarkUploads", id: bookmark.id) try transaction.set(UUID().uuidString, in: "bookmarkDeletes", id: bookmark.id) } - try load() + try await load() if let uid = activeUID, let cloudStore { await deleteFromCloud(bookmark.id, uid: uid, cloudStore: cloudStore) } } catch { errorMessage = error.localizedDescription } } func link(_ bookmark: RepositoryBookmark, to repositoryURL: URL) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in try transaction.set(repositoryURL.path, in: "bookmarkPaths", id: bookmark.id) } - try load() + try await load() } func validateAndLink(_ bookmark: RepositoryBookmark, to repositoryURL: URL) async throws { @@ -253,7 +264,7 @@ final class RepositoryBookmarkController: ObservableObject { let matches = bookmarks.filter { localPaths[$0.id] == nil && keys.contains($0.canonicalKey) } guard !matches.isEmpty else { continue } do { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in for bookmark in matches { // Recheck persisted state: a cloud update or manual link may have won the race. guard let current = try transaction.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id), @@ -262,15 +273,15 @@ final class RepositoryBookmarkController: ObservableObject { try transaction.set(repositoryURL.path, in: "bookmarkPaths", id: bookmark.id) } } - try load() + try await load() } catch { errorMessage = error.localizedDescription } } } func unlinkLocalFolder(for bookmark: RepositoryBookmark) async { do { - try await dataStore.transaction { $0.remove(in: "bookmarkPaths", id: bookmark.id) } - try load() + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { $0.remove(in: "bookmarkPaths", id: bookmark.id) } + try await load() } catch { errorMessage = error.localizedDescription } } @@ -287,12 +298,12 @@ final class RepositoryBookmarkController: ObservableObject { } private func acknowledge(_ collection: String, id: String, version: String, uid: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in guard self.activeUID == uid, try transaction.value(String.self, in: collection, id: id) == version else { return } transaction.remove(in: collection, id: id) } - try load() + try await load() } private func upload(_ bookmark: RepositoryBookmark, uid: String, cloudStore: RepositoryBookmarkCloudStore) async { @@ -300,7 +311,7 @@ final class RepositoryBookmarkController: ObservableObject { syncingBookmarkIDs.insert(bookmark.id) defer { syncingBookmarkIDs.remove(bookmark.id) } do { - guard let version = try dataStore.value(String.self, in: "bookmarkUploads", id: bookmark.id) else { return } + guard let version = try await dataStore.readValue(String.self, in: "bookmarkUploads", id: bookmark.id) else { return } try await cloudStore.save(bookmark, uid: uid) try await acknowledge("bookmarkUploads", id: bookmark.id, version: version, uid: uid) } catch { errorMessage = error.localizedDescription } @@ -311,7 +322,7 @@ final class RepositoryBookmarkController: ObservableObject { syncingBookmarkIDs.insert(id) defer { syncingBookmarkIDs.remove(id) } do { - guard let version = try dataStore.value(String.self, in: "bookmarkDeletes", id: id) else { return } + guard let version = try await dataStore.readValue(String.self, in: "bookmarkDeletes", id: id) else { return } try await cloudStore.delete(bookmarkID: id, uid: uid) try await acknowledge("bookmarkDeletes", id: id, version: version, uid: uid) } catch { errorMessage = error.localizedDescription } @@ -325,8 +336,8 @@ final class RepositoryBookmarkController: ObservableObject { await upload(bookmark, uid: uid, cloudStore: cloudStore) } // A replacement must reach the cloud before deleting its old identity. - guard try dataStore.values(String.self, in: "bookmarkUploads").isEmpty else { return } - for id in try dataStore.values(String.self, in: "bookmarkDeletes").keys { + guard try await dataStore.readValues(String.self, in: "bookmarkUploads").isEmpty else { return } + for id in try await dataStore.readValues(String.self, in: "bookmarkDeletes").keys { guard activeUID == uid else { return } if syncingBookmarkIDs.contains(id) { continue } await deleteFromCloud(id, uid: uid, cloudStore: cloudStore) @@ -335,7 +346,7 @@ final class RepositoryBookmarkController: ObservableObject { } private func applyCloudBookmarks(_ cloudBookmarks: [RepositoryBookmark], uid: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["bookmarks", "bookmarkPaths", "bookmarkUploads", "bookmarkDeletes"]) { transaction in guard self.activeUID == uid else { return } let pendingUploads = try transaction.values(String.self, in: "bookmarkUploads") let pendingDeletes = try transaction.values(String.self, in: "bookmarkDeletes") @@ -352,6 +363,6 @@ final class RepositoryBookmarkController: ObservableObject { transaction.remove(in: "bookmarkPaths", id: id) } } - try load() + try await load() } } diff --git a/macgit/App/RepositoryCommitRuleSyncController.swift b/macgit/App/RepositoryCommitRuleSyncController.swift index b33b9e1..4bf677a 100644 --- a/macgit/App/RepositoryCommitRuleSyncController.swift +++ b/macgit/App/RepositoryCommitRuleSyncController.swift @@ -29,7 +29,7 @@ final class RepositoryCommitRuleSyncController: ObservableObject { func markChanged(_ value: Bool, uid: String?, repositoryURL: URL) async throws { guard let uid else { return } let id = key(uid: uid, path: repositoryURL.path) - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["repoSettings", "commitRulePending"]) { transaction in try transaction.set(value, in: "commitRulePending", id: id) } } @@ -46,12 +46,13 @@ final class RepositoryCommitRuleSyncController: ObservableObject { do { try await dataStore.prepare() guard session == sessionID else { return nil } - let initial = localValue(repositoryURL) + let initial = await localValue(repositoryURL) + guard session == sessionID else { return nil } let remote = try await cloud.load(identity: identity, uid: uid) guard session == sessionID else { return nil } // Edits made while offline or while loading always win over an older download. - if pendingValues[pendingID] == nil, let remote { - let applied = try await dataStore.transaction { transaction in + if (try await pendingValue(pendingID)) == nil, let remote { + let applied = try await dataStore.transaction(reading: ["repoSettings", "commitRulePending"]) { transaction in guard session == self.sessionID, try transaction.value(Bool.self, in: "commitRulePending", id: pendingID) == nil else { return false } var settings = try transaction.value(RepoSettings.self, in: "repoSettings", id: repositoryURL.path) @@ -61,16 +62,19 @@ final class RepositoryCommitRuleSyncController: ObservableObject { try transaction.set(settings, in: "repoSettings", id: repositoryURL.path) return true } - if applied, session == sessionID, pendingValues[pendingID] == nil, localValue(repositoryURL) == remote { + if applied, session == sessionID, (try await pendingValue(pendingID)) == nil, await localValue(repositoryURL) == remote { + guard session == sessionID else { return nil } onApplied(remote) } - } else if pendingValues[pendingID] == nil { + } else if (try await pendingValue(pendingID)) == nil { + guard session == sessionID else { return nil } try await markChanged(initial, uid: uid, repositoryURL: repositoryURL) } - while session == sessionID, let value = pendingValues[pendingID] { + while session == sessionID, let value = (try await pendingValue(pendingID)) { + guard session == sessionID else { return nil } try await cloud.save(value, identity: identity, uid: uid) guard session == sessionID else { return nil } - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["repoSettings", "commitRulePending"]) { transaction in guard session == self.sessionID, try transaction.value(Bool.self, in: "commitRulePending", id: pendingID) == value else { return } transaction.remove(in: "commitRulePending", id: pendingID) @@ -83,12 +87,12 @@ final class RepositoryCommitRuleSyncController: ObservableObject { } } - private var pendingValues: [String: Bool] { - (try? dataStore.values(Bool.self, in: "commitRulePending")) ?? [:] + private func pendingValue(_ id: String) async throws -> Bool? { + try await dataStore.readValue(Bool.self, in: "commitRulePending", id: id) } private func key(uid: String, path: String) -> String { "\(uid)|\(path)" } - private func localValue(_ url: URL) -> Bool { - localStore.settings(for: url.path, currentBranch: nil, remotes: []).skipProtectedBranchCommitWarnings + private func localValue(_ url: URL) async -> Bool { + await localStore.settings(for: url.path, currentBranch: nil, remotes: []).skipProtectedBranchCommitWarnings } } diff --git a/macgit/App/RepositoryVisibilityController.swift b/macgit/App/RepositoryVisibilityController.swift index d3a782d..b8008e3 100644 --- a/macgit/App/RepositoryVisibilityController.swift +++ b/macgit/App/RepositoryVisibilityController.swift @@ -140,7 +140,7 @@ final class RepositoryVisibilityController: ObservableObject { forceRefresh: Bool ) async -> RepositoryVisibility { if !forceRefresh, - let cached = cache.cachedVisibility( + let cached = await cache.cachedVisibility( for: repository, maximumAge: cacheMaximumAge, now: .now diff --git a/macgit/Services/GitProviderAccountPreferenceStore.swift b/macgit/Services/GitProviderAccountPreferenceStore.swift index e85b6d6..f334503 100644 --- a/macgit/Services/GitProviderAccountPreferenceStore.swift +++ b/macgit/Services/GitProviderAccountPreferenceStore.swift @@ -51,7 +51,7 @@ final class GitProviderAccountPreferenceStore { } func update(accountID: String?, forPreferenceKey preferenceKey: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerPreferences"]) { transaction in if let accountID, !accountID.isEmpty { try transaction.set(accountID, in: "providerPreferences", id: preferenceKey) } else { @@ -61,7 +61,7 @@ final class GitProviderAccountPreferenceStore { } func replacePreferences(_ preferences: [String: String]) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerPreferences"]) { transaction in for key in try transaction.values(String.self, in: "providerPreferences").keys { transaction.remove(in: "providerPreferences", id: key) } diff --git a/macgit/Services/LocalDataError.swift b/macgit/Services/LocalDataError.swift index 3e9db79..b78464b 100644 --- a/macgit/Services/LocalDataError.swift +++ b/macgit/Services/LocalDataError.swift @@ -3,9 +3,11 @@ import Foundation enum LocalDataError: LocalizedError { case notReady + case collectionNotLoaded(String) case invalidLegacyData(String) var errorDescription: String? { switch self { + case .collectionNotLoaded(let collection): "Local collection requires an explicit read scope: \(collection)." case .notReady: "Local data is still loading." case .invalidLegacyData(let key): "Saved data could not be migrated (\(key)). The original data has been retained." } diff --git a/macgit/Services/LocalDataStore.swift b/macgit/Services/LocalDataStore.swift index 0b5db19..5627729 100644 --- a/macgit/Services/LocalDataStore.swift +++ b/macgit/Services/LocalDataStore.swift @@ -2,7 +2,8 @@ import Combine import Foundation -/// UI reads use this snapshot. SQLite I/O is isolated to LocalSQLiteDatabase. +/// Only credential-routing metadata stays resident for synchronous callers. +/// Other data is read on demand; SQLite I/O is isolated to LocalSQLiteDatabase. /// All writers share one queue, including transactions spanning several stores. @MainActor final class LocalDataStore: ObservableObject { @@ -21,7 +22,9 @@ final class LocalDataStore: ObservableObject { @Published private(set) var errorMessage: String? private let database: LocalSQLiteDatabase private let defaults: UserDefaults + private let residentCollections: Set = ["providerAccounts", "providerPreferences", "sshPaths"] private var records: [String: [String: Data]] = [:] + var residentCollectionNames: Set { Set(records.keys) } private var loading: Task? private var writer: Task? @@ -37,7 +40,8 @@ final class LocalDataStore: ObservableObject { let task = Task { @MainActor in let needsImport = try await database.needsLegacyImport() let legacy = needsImport ? try LegacyLocalDataMigration.records(from: defaults) : nil - records = try await database.load(importing: legacy) + try await database.prepare(importing: legacy) + records = try await database.read(collections: residentCollections) isReady = true errorMessage = nil } @@ -54,23 +58,45 @@ final class LocalDataStore: ObservableObject { func value(_ type: T.Type, in collection: String, id: String) throws -> T? { guard isReady else { throw LocalDataError.notReady } + guard residentCollections.contains(collection) else { throw LocalDataError.collectionNotLoaded(collection) } return try records[collection]?[id].map { try JSONDecoder().decode(type, from: $0) } } func values(_ type: T.Type, in collection: String) throws -> [String: T] { guard isReady else { throw LocalDataError.notReady } + guard residentCollections.contains(collection) else { throw LocalDataError.collectionNotLoaded(collection) } return try (records[collection] ?? [:]).mapValues { try JSONDecoder().decode(type, from: $0) } } - func transaction(_ update: @escaping (inout LocalDataTransaction) throws -> T) async throws -> T { + func readValue(_ type: T.Type, in collection: String, id: String) async throws -> T? { + await writer?.value + try await prepare() + let data = try await database.value(in: collection, id: id) + return try data.map { try JSONDecoder().decode(type, from: $0) } + } + + func readValues(_ type: T.Type, in collection: String) async throws -> [String: T] { + let snapshot = try await snapshot(collections: [collection]) + return try snapshot.values(type, in: collection) + } + + func snapshot(collections: Set) async throws -> LocalDataTransaction { + await writer?.value + try await prepare() + return LocalDataTransaction(records: try await database.read(collections: collections)) + } + + func transaction(reading collections: Set = [], _ update: @escaping (inout LocalDataTransaction) throws -> T) async throws -> T { let previous = writer let task = Task { @MainActor in await previous?.value try await prepare() - var transaction = LocalDataTransaction(records: records) + var transaction = LocalDataTransaction(records: try await database.read(collections: collections)) let result = try update(&transaction) try await database.commit(transaction.changes) - records = transaction.records + for (collection, id, data) in transaction.changes where residentCollections.contains(collection) { + records[collection, default: [:]][id] = data + } return result } writer = Task { _ = try? await task.value } diff --git a/macgit/Services/LocalDataTransaction.swift b/macgit/Services/LocalDataTransaction.swift index 5cb4d6e..dc525cc 100644 --- a/macgit/Services/LocalDataTransaction.swift +++ b/macgit/Services/LocalDataTransaction.swift @@ -6,11 +6,13 @@ struct LocalDataTransaction { private(set) var changes: [(String, String, Data?)] = [] func value(_ type: T.Type, in collection: String, id: String) throws -> T? { - try records[collection]?[id].map { try JSONDecoder().decode(type, from: $0) } + guard records[collection] != nil else { throw LocalDataError.collectionNotLoaded(collection) } + return try records[collection]?[id].map { try JSONDecoder().decode(type, from: $0) } } func values(_ type: T.Type, in collection: String) throws -> [String: T] { - try (records[collection] ?? [:]).mapValues { try JSONDecoder().decode(type, from: $0) } + guard records[collection] != nil else { throw LocalDataError.collectionNotLoaded(collection) } + return try (records[collection] ?? [:]).mapValues { try JSONDecoder().decode(type, from: $0) } } mutating func set(_ value: T, in collection: String, id: String) throws { diff --git a/macgit/Services/LocalGitProviderAccountStore.swift b/macgit/Services/LocalGitProviderAccountStore.swift index 4f1e312..7315f05 100644 --- a/macgit/Services/LocalGitProviderAccountStore.swift +++ b/macgit/Services/LocalGitProviderAccountStore.swift @@ -26,9 +26,9 @@ protocol GitProviderAccountLocalStore { func save(_ account: GitProviderAccount) async throws func delete(accountID: String) async throws -> GitProviderAccount? func remove(accountID: String) async throws - func pendingDeletions() throws -> [GitProviderAccount] + func pendingDeletions() async throws -> [GitProviderAccount] func clearPendingDeletion(_ account: GitProviderAccount) async throws - func syncedIdentityKeys(uid: String) -> Set + func syncedIdentityKeys(uid: String) async -> Set func setSyncedIdentityKeys(_ keys: Set, uid: String) async throws } @@ -53,7 +53,7 @@ final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { } func save(_ account: GitProviderAccount) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerAccounts", "providerDeletions", "providerPreferences"]) { transaction in let identity = GitProviderAccountLocalIdentity(account) for (id, stored) in try transaction.values(GitProviderAccount.self, in: "providerAccounts") where id == account.id || GitProviderAccountLocalIdentity(stored) == identity { @@ -68,7 +68,7 @@ final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { } func delete(accountID: String) async throws -> GitProviderAccount? { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerAccounts", "providerDeletions", "providerPreferences"]) { transaction in guard let account = try transaction.value(GitProviderAccount.self, in: "providerAccounts", id: accountID) else { return nil } try Self.remove(account, from: &transaction) for (id, deleted) in try transaction.values(GitProviderAccount.self, in: "providerDeletions") @@ -81,7 +81,7 @@ final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { } func remove(accountID: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerAccounts", "providerDeletions", "providerPreferences"]) { transaction in if let account = try transaction.value(GitProviderAccount.self, in: "providerAccounts", id: accountID) { try Self.remove(account, from: &transaction) } @@ -96,12 +96,12 @@ final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { } } - func pendingDeletions() throws -> [GitProviderAccount] { - Array(try dataStore.values(GitProviderAccount.self, in: "providerDeletions").values) + func pendingDeletions() async throws -> [GitProviderAccount] { + Array(try await dataStore.readValues(GitProviderAccount.self, in: "providerDeletions").values) } func clearPendingDeletion(_ account: GitProviderAccount) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerAccounts", "providerDeletions", "providerPreferences"]) { transaction in for (id, deleted) in try transaction.values(GitProviderAccount.self, in: "providerDeletions") where GitProviderAccountLocalIdentity(deleted) == GitProviderAccountLocalIdentity(account) { transaction.remove(in: "providerDeletions", id: id) @@ -109,12 +109,12 @@ final class SQLiteGitProviderAccountLocalStore: GitProviderAccountLocalStore { } } - func syncedIdentityKeys(uid: String) -> Set { - Set((try? dataStore.value([String].self, in: "providerSyncedIdentities", id: uid)) ?? []) + func syncedIdentityKeys(uid: String) async -> Set { + Set((try? await dataStore.readValue([String].self, in: "providerSyncedIdentities", id: uid)) ?? []) } func setSyncedIdentityKeys(_ keys: Set, uid: String) async throws { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["providerAccounts", "providerDeletions", "providerPreferences"]) { transaction in try transaction.set(keys.sorted(), in: "providerSyncedIdentities", id: uid) } } @@ -154,7 +154,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { return } - let pendingDeletions = try localStore.pendingDeletions() + let pendingDeletions = try await localStore.pendingDeletions() let pendingIdentities = Set(pendingDeletions.map(GitProviderAccountLocalIdentity.init)) for deletedAccount in pendingDeletions { let identity = GitProviderAccountLocalIdentity(deletedAccount) @@ -176,7 +176,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { let visibleCloudIdentityKeys = Set(visibleCloudAccounts.map { GitProviderAccountLocalIdentity($0).storageKey }) - let previouslySyncedIdentityKeys = localStore.syncedIdentityKeys(uid: uid) + let previouslySyncedIdentityKeys = await localStore.syncedIdentityKeys(uid: uid) for localAccount in initialLocalAccounts { let identityKey = GitProviderAccountLocalIdentity(localAccount).storageKey if previouslySyncedIdentityKeys.contains(identityKey), @@ -213,7 +213,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { try await localStore.save(account) guard let cloudUID else { return } if await mirrorToCloud(account, uid: cloudUID) { - var syncedKeys = localStore.syncedIdentityKeys(uid: cloudUID) + var syncedKeys = await localStore.syncedIdentityKeys(uid: cloudUID) syncedKeys.insert(GitProviderAccountLocalIdentity(account).storageKey) // The account is already durable. A sync bookkeeping failure must not // make the caller delete credentials as if the local save had failed. @@ -232,7 +232,7 @@ final class LocalFirstGitProviderAccountStore: GitProviderAccountStore { try? await cloudStore.delete(accountID: accountID, macgitUID: cloudUID) } try await localStore.clearPendingDeletion(deletedAccount) - var syncedKeys = localStore.syncedIdentityKeys(uid: cloudUID) + var syncedKeys = await localStore.syncedIdentityKeys(uid: cloudUID) syncedKeys.remove(GitProviderAccountLocalIdentity(deletedAccount).storageKey) try await localStore.setSyncedIdentityKeys(syncedKeys, uid: cloudUID) } catch { diff --git a/macgit/Services/LocalSQLiteDatabase.swift b/macgit/Services/LocalSQLiteDatabase.swift index 5c3d95d..dbfb672 100644 --- a/macgit/Services/LocalSQLiteDatabase.swift +++ b/macgit/Services/LocalSQLiteDatabase.swift @@ -12,7 +12,7 @@ actor LocalSQLiteDatabase { try withDatabase { db in try !hasImported(db) } } - func load(importing legacy: [String: [String: Data]]?) throws -> [String: [String: Data]] { + func prepare(importing legacy: [String: [String: Data]]?) throws { try withDatabase { db in if let legacy { try transaction(db) { @@ -23,17 +23,17 @@ actor LocalSQLiteDatabase { } } // Read back every imported row before marking the import complete. - let imported = try read(db) for (collection, values) in legacy { - for (id, payload) in values where imported[collection]?[id] != payload { - throw failure("Local data migration verification failed.") + for (id, payload) in values { + guard try readValue(db, collection: collection, id: id) == payload else { + throw failure("Local data migration verification failed.") + } } } try query(db, "INSERT INTO migrations (id) VALUES ('user-defaults-v1')") } } } - return try read(db) } } @@ -60,10 +60,30 @@ actor LocalSQLiteDatabase { [collection, id, String(decoding: payload, as: UTF8.self)]) } - private func read(_ db: OpaquePointer) throws -> [String: [String: Data]] { - var result: [String: [String: Data]] = [:] - try query(db, "SELECT collection, id, payload FROM records ORDER BY collection, id") { row in - result[column(row, 0), default: [:]][column(row, 1)] = Data(column(row, 2).utf8) + func value(in collection: String, id: String) throws -> Data? { + try withDatabase { try readValue($0, collection: collection, id: id) } + } + + func read(collections: Set) throws -> [String: [String: Data]] { + guard !collections.isEmpty else { return [:] } + return try withDatabase { db in + var result: [String: [String: Data]] = [:] + try transaction(db) { + for collection in collections { + result[collection] = [:] + try query(db, "SELECT id, payload FROM records WHERE collection = ?", [collection]) { row in + result[collection]?[column(row, 0)] = Data(column(row, 1).utf8) + } + } + } + return result + } + } + + private func readValue(_ db: OpaquePointer, collection: String, id: String) throws -> Data? { + var result: Data? + try query(db, "SELECT payload FROM records WHERE collection = ? AND id = ?", [collection, id]) { + result = Data(column($0, 0).utf8) } return result } diff --git a/macgit/Services/RepoSettingsStore.swift b/macgit/Services/RepoSettingsStore.swift index 84ec2d6..958dc3c 100644 --- a/macgit/Services/RepoSettingsStore.swift +++ b/macgit/Services/RepoSettingsStore.swift @@ -29,8 +29,8 @@ final class RepoSettingsStore { init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } - func settings(for repositoryPath: String, currentBranch: String?, remotes: [String]) -> RepoSettings { - (try? dataStore.value(RepoSettings.self, in: "repoSettings", id: repositoryPath)) + func settings(for repositoryPath: String, currentBranch: String?, remotes: [String]) async -> RepoSettings { + (try? await dataStore.readValue(RepoSettings.self, in: "repoSettings", id: repositoryPath)) ?? RepoSettings.defaults(currentBranch: currentBranch, remotes: remotes) } diff --git a/macgit/Services/RepositoryVisibilityCache.swift b/macgit/Services/RepositoryVisibilityCache.swift index f35de4e..c9a8356 100644 --- a/macgit/Services/RepositoryVisibilityCache.swift +++ b/macgit/Services/RepositoryVisibilityCache.swift @@ -63,7 +63,7 @@ protocol RepositoryVisibilityCaching { for repository: GitRepositoryIdentity, maximumAge: TimeInterval, now: Date - ) -> RepositoryVisibility? + ) async -> RepositoryVisibility? func save( _ visibility: RepositoryVisibility, @@ -77,8 +77,8 @@ final class SQLiteRepositoryVisibilityCache: RepositoryVisibilityCaching { private let dataStore: LocalDataStore init(dataStore: LocalDataStore? = nil) { self.dataStore = dataStore ?? .shared } - func cachedVisibility(for repository: GitRepositoryIdentity, maximumAge: TimeInterval, now: Date) -> RepositoryVisibility? { - guard let record = try? dataStore.value(CachedRepositoryVisibility.self, in: "repositoryVisibility", + func cachedVisibility(for repository: GitRepositoryIdentity, maximumAge: TimeInterval, now: Date) async -> RepositoryVisibility? { + guard let record = try? await dataStore.readValue(CachedRepositoryVisibility.self, in: "repositoryVisibility", id: CachedRepositoryVisibility.cacheKey(for: repository)), record.visibility == .public || record.visibility == .private, now.timeIntervalSince(record.resolvedAt) >= 0, @@ -90,7 +90,7 @@ final class SQLiteRepositoryVisibilityCache: RepositoryVisibilityCaching { guard visibility == .public || visibility == .private else { return } let record = CachedRepositoryVisibility(repository: repository, visibility: visibility, resolvedAt: resolvedAt) do { - try await dataStore.transaction { transaction in + try await dataStore.transaction(reading: ["repositoryVisibility"]) { transaction in for (id, cached) in try transaction.values(CachedRepositoryVisibility.self, in: "repositoryVisibility") where resolvedAt.timeIntervalSince(cached.resolvedAt) > 30 * 24 * 60 * 60 { transaction.remove(in: "repositoryVisibility", id: id) diff --git a/macgit/Views/MainWindow/MainWindowView.swift b/macgit/Views/MainWindow/MainWindowView.swift index 950bafc..5849591 100644 --- a/macgit/Views/MainWindow/MainWindowView.swift +++ b/macgit/Views/MainWindow/MainWindowView.swift @@ -1645,7 +1645,7 @@ struct MainWindowView: View { loadedGitFlowCheckpoint, loadedGitCommonDirectory ) - let loadedSettings = repoSettingsStore.settings( + let loadedSettings = await repoSettingsStore.settings( for: repositoryURL.path, currentBranch: currentBranch, remotes: remotes diff --git a/macgitTests/GitFlowConfigurationSyncTests.swift b/macgitTests/GitFlowConfigurationSyncTests.swift index e40577d..b262ec0 100644 --- a/macgitTests/GitFlowConfigurationSyncTests.swift +++ b/macgitTests/GitFlowConfigurationSyncTests.swift @@ -242,7 +242,8 @@ final class GitFlowConfigurationSyncTests: XCTestCase { XCTAssertEqual(cloudStore.configurationRequestCount, 0) XCTAssertEqual(cloudStore.savedConfiguration?.mainBranch, "trunk") XCTAssertEqual(cloudStore.savedConfiguration?.developBranch, "next") - XCTAssertTrue(try fixture.store.values(String.self, in: "gitFlowPending").isEmpty) + let loadedValue1 = try await fixture.store.readValues(String.self, in: "gitFlowPending") + XCTAssertTrue(loadedValue1.isEmpty) } private func makeRepository() throws -> URL { diff --git a/macgitTests/LocalDataMigrationTests.swift b/macgitTests/LocalDataMigrationTests.swift index d64b9a3..3d7d39c 100644 --- a/macgitTests/LocalDataMigrationTests.swift +++ b/macgitTests/LocalDataMigrationTests.swift @@ -32,22 +32,35 @@ final class LocalDataMigrationTests: XCTestCase { try await fixture.store.prepare() let reopened = try await fixture.reopen() - XCTAssertEqual(try reopened.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id), bookmark) - XCTAssertEqual(try reopened.value(String.self, in: "bookmarkPaths", id: bookmark.id), "/tmp/repo") - XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkUploads", id: bookmark.id)) - XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkDeletes", id: "deleted-bookmark")) - XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id), account) - XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerDeletions", id: account.id), account) - XCTAssertEqual(try reopened.value([String].self, in: "providerSyncedIdentities", id: "user-a"), ["github|github.com|42"]) - XCTAssertEqual(try reopened.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) - XCTAssertEqual(try reopened.value(String.self, in: "providerPreferences", id: "remote"), account.id) - XCTAssertEqual(try reopened.value(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)), ssh) - XCTAssertEqual(try reopened.value(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo"), false) - XCTAssertNotNil(try reopened.value(String.self, in: "gitFlowPending", id: "user-a|remote")) + let loadedValue1 = try await reopened.readValue(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id) + XCTAssertEqual(loadedValue1, bookmark) + let loadedValue2 = try await reopened.readValue(String.self, in: "bookmarkPaths", id: bookmark.id) + XCTAssertEqual(loadedValue2, "/tmp/repo") + let loadedValue3 = try await reopened.readValue(String.self, in: "bookmarkUploads", id: bookmark.id) + XCTAssertNotNil(loadedValue3) + let loadedValue4 = try await reopened.readValue(String.self, in: "bookmarkDeletes", id: "deleted-bookmark") + XCTAssertNotNil(loadedValue4) + let loadedValue5 = try await reopened.readValue(GitProviderAccount.self, in: "providerAccounts", id: account.id) + XCTAssertEqual(loadedValue5, account) + let loadedValue6 = try await reopened.readValue(GitProviderAccount.self, in: "providerDeletions", id: account.id) + XCTAssertEqual(loadedValue6, account) + let loadedValue7 = try await reopened.readValue([String].self, in: "providerSyncedIdentities", id: "user-a") + XCTAssertEqual(loadedValue7, ["github|github.com|42"]) + let loadedValue8 = try await reopened.readValue(RepoSettings.self, in: "repoSettings", id: "/tmp/repo") + XCTAssertEqual(loadedValue8, settings) + let loadedValue9 = try await reopened.readValue(String.self, in: "providerPreferences", id: "remote") + XCTAssertEqual(loadedValue9, account.id) + let loadedValue10 = try await reopened.readValue(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) + XCTAssertEqual(loadedValue10, ssh) + let loadedValue11 = try await reopened.readValue(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo") + XCTAssertEqual(loadedValue11, false) + let loadedValue12 = try await reopened.readValue(String.self, in: "gitFlowPending", id: "user-a|remote") + XCTAssertNotNil(loadedValue12) XCTAssertEqual(SQLiteGitProviderAccountLocalStore(dataStore: reopened, defaults: fixture.defaults).accountOwnerID, "original-owner") XCTAssertEqual(fixture.defaults.data(forKey: prefix + "localGitProviderAccounts"), encodedAccounts) XCTAssertEqual(fixture.defaults.string(forKey: "appearance"), "dark") - XCTAssertTrue(try reopened.values(String.self, in: "appearance").isEmpty) + let loadedValue13 = try await reopened.readValues(String.self, in: "appearance") + XCTAssertTrue(loadedValue13.isEmpty) } func testCompletedImportDoesNotResurrectDeletedAccountFromDefaults() async throws { @@ -59,8 +72,10 @@ final class LocalDataMigrationTests: XCTestCase { let accounts = SQLiteGitProviderAccountLocalStore(dataStore: fixture.store, defaults: fixture.defaults) _ = try await accounts.delete(accountID: account.id) let reopened = try await fixture.reopen() - XCTAssertNil(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id)) - XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerDeletions", id: account.id), account) + let loadedValue14 = try await reopened.readValue(GitProviderAccount.self, in: "providerAccounts", id: account.id) + XCTAssertNil(loadedValue14) + let loadedValue15 = try await reopened.readValue(GitProviderAccount.self, in: "providerDeletions", id: account.id) + XCTAssertEqual(loadedValue15, account) // Stale or subsequently damaged defaults must no longer be consulted. fixture.defaults.set(Data("broken".utf8), forKey: "dev.thanhtran.macgit.localGitProviderAccounts") _ = try await fixture.reopen() @@ -78,7 +93,8 @@ final class LocalDataMigrationTests: XCTestCase { let settings = RepoSettings(defaultPullBranch: "recovered") fixture.defaults.set(try JSONEncoder().encode(["/tmp/repo": settings]), forKey: key) try await fixture.store.prepare() - XCTAssertEqual(try fixture.store.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) + let loadedValue16 = try await fixture.store.readValue(RepoSettings.self, in: "repoSettings", id: "/tmp/repo") + XCTAssertEqual(loadedValue16, settings) } func testAccountDeleteRollsBackMetadataLinksAndTombstoneOnDiskFailure() async throws { @@ -95,16 +111,22 @@ final class LocalDataMigrationTests: XCTestCase { do { _ = try await accounts.delete(accountID: account.id); XCTFail("Write should fail") } catch { } XCTAssertEqual(try accounts.accounts(), [account]) - XCTAssertTrue(try accounts.pendingDeletions().isEmpty) + let pendingBeforeDelete = try await accounts.pendingDeletions() + XCTAssertTrue(pendingBeforeDelete.isEmpty) let reopened = try await fixture.reopen() - XCTAssertEqual(try reopened.value(GitProviderAccount.self, in: "providerAccounts", id: account.id), account) - XCTAssertEqual(try reopened.value(String.self, in: "providerPreferences", id: "remote"), account.id) - XCTAssertNotNil(try reopened.value(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account))) + let loadedValue17 = try await reopened.readValue(GitProviderAccount.self, in: "providerAccounts", id: account.id) + XCTAssertEqual(loadedValue17, account) + let loadedValue18 = try await reopened.readValue(String.self, in: "providerPreferences", id: "remote") + XCTAssertEqual(loadedValue18, account.id) + let loadedValue19 = try await reopened.readValue(GitProviderSSHKey.self, in: "sshPaths", id: GitProviderSSHKeyStoreKey.storageKey(for: account)) + XCTAssertNotNil(loadedValue19) try execute("DROP TRIGGER fail_delete", url: fixture.databaseURL) _ = try await accounts.delete(accountID: account.id) XCTAssertNil(try keys.key(for: account)) - XCTAssertTrue(try fixture.store.values(String.self, in: "providerPreferences").isEmpty) - XCTAssertEqual(try accounts.pendingDeletions(), [account]) + let loadedValue20 = try await fixture.store.readValues(String.self, in: "providerPreferences") + XCTAssertTrue(loadedValue20.isEmpty) + let pendingAfterDelete = try await accounts.pendingDeletions() + XCTAssertEqual(pendingAfterDelete, [account]) } func testFailedImportRollsBackRowsAndMarkerThenRetries() async throws { @@ -117,11 +139,12 @@ final class LocalDataMigrationTests: XCTestCase { fixture.defaults.set(try JSONEncoder().encode([makeAccount()]), forKey: "dev.thanhtran.macgit.localGitProviderAccounts") do { try await fixture.store.prepare(); XCTFail("Import should fail") } catch { } - let rows = try await engine.load(importing: nil) - XCTAssertTrue(rows.isEmpty) + let rows = try await engine.read(collections: ["providerAccounts"]) + XCTAssertTrue(rows.values.allSatisfy(\.isEmpty)) try execute("DROP TRIGGER fail_import", url: fixture.databaseURL) try await fixture.store.prepare() - XCTAssertEqual(try fixture.store.values(GitProviderAccount.self, in: "providerAccounts").count, 1) + let loadedValue21 = try await fixture.store.readValues(GitProviderAccount.self, in: "providerAccounts") + XCTAssertEqual(loadedValue21.count, 1) } func testConcurrentTransactionsPreserveBothUpdates() async throws { @@ -133,7 +156,8 @@ final class LocalDataMigrationTests: XCTestCase { try await first.value try await second.value let reopened = try await fixture.reopen() - XCTAssertEqual(try reopened.values(Int.self, in: "counter"), ["first": 1, "second": 2]) + let loadedValue22 = try await reopened.readValues(Int.self, in: "counter") + XCTAssertEqual(loadedValue22, ["first": 1, "second": 2]) } func testRepositorySettingsAndPendingRuleCommitTogether() async throws { @@ -144,9 +168,12 @@ final class LocalDataMigrationTests: XCTestCase { settings.skipProtectedBranchCommitWarnings = true try await RepoSettingsStore(dataStore: fixture.store).update(for: "/tmp/repo", settings: settings, pendingCommitRuleUID: "user-a") let reopened = try await fixture.reopen() - XCTAssertEqual(try reopened.value(RepoSettings.self, in: "repoSettings", id: "/tmp/repo"), settings) - XCTAssertEqual(try reopened.value(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo"), true) - XCTAssertNil(try reopened.value(Bool.self, in: "commitRulePending", id: "user-b|/tmp/repo")) + let loadedValue23 = try await reopened.readValue(RepoSettings.self, in: "repoSettings", id: "/tmp/repo") + XCTAssertEqual(loadedValue23, settings) + let loadedValue24 = try await reopened.readValue(Bool.self, in: "commitRulePending", id: "user-a|/tmp/repo") + XCTAssertEqual(loadedValue24, true) + let loadedValue25 = try await reopened.readValue(Bool.self, in: "commitRulePending", id: "user-b|/tmp/repo") + XCTAssertNil(loadedValue25) } func testBookmarkRemovalRollsBackAllRelatedRowsOnFailure() async throws { @@ -161,16 +188,19 @@ final class LocalDataMigrationTests: XCTestCase { try transaction.set("pending", in: "bookmarkUploads", id: bookmark.id) } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() try execute("CREATE TRIGGER fail_bookmark BEFORE INSERT ON records WHEN NEW.collection = 'bookmarkDeletes' BEGIN SELECT RAISE(ABORT, 'test failure'); END", url: fixture.databaseURL) await controller.removeBookmark(bookmark) XCTAssertNotNil(controller.errorMessage) XCTAssertEqual(controller.bookmarks, [bookmark]) XCTAssertEqual(controller.localURL(for: bookmark)?.path, "/tmp/repo") let reopened = try await fixture.reopen() - XCTAssertNotNil(try reopened.value(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id)) - XCTAssertEqual(try reopened.value(String.self, in: "bookmarkUploads", id: bookmark.id), "pending") - XCTAssertNil(try reopened.value(String.self, in: "bookmarkDeletes", id: bookmark.id)) + let loadedValue26 = try await reopened.readValue(RepositoryBookmark.self, in: "bookmarks", id: bookmark.id) + XCTAssertNotNil(loadedValue26) + let loadedValue27 = try await reopened.readValue(String.self, in: "bookmarkUploads", id: bookmark.id) + XCTAssertEqual(loadedValue27, "pending") + let loadedValue28 = try await reopened.readValue(String.self, in: "bookmarkDeletes", id: bookmark.id) + XCTAssertNil(loadedValue28) } func testBookmarkCloudFailureKeepsDeletionAcrossReopenAndStaleSnapshot() async throws { @@ -195,7 +225,8 @@ final class LocalDataMigrationTests: XCTestCase { await second.updateAccount(account) XCTAssertTrue(second.bookmarks.isEmpty) XCTAssertNil(second.localURL(for: bookmark)) - XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkDeletes", id: bookmark.id)) + let loadedValue29 = try await reopened.readValue(String.self, in: "bookmarkDeletes", id: bookmark.id) + XCTAssertNotNil(loadedValue29) } private func execute(_ sql: String, url: URL) throws { diff --git a/macgitTests/LocalDataOnDemandTests.swift b/macgitTests/LocalDataOnDemandTests.swift new file mode 100644 index 0000000..e24b481 --- /dev/null +++ b/macgitTests/LocalDataOnDemandTests.swift @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +@MainActor +final class LocalDataOnDemandTests: XCTestCase { + func testPrepareAndOnDemandReadsDoNotRetainUnrelatedPayloads() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.prepare() + try await fixture.store.transaction { + try $0.set(String(repeating: "large", count: 100_000), in: "unrelated", id: "large") + try $0.set("selected", in: "repoSettings", id: "/selected") + } + let reopened = try await fixture.reopen() + let resident: Set = ["providerAccounts", "providerPreferences", "sshPaths"] + XCTAssertEqual(reopened.residentCollectionNames, resident) + let selected = try await reopened.readValue(String.self, in: "repoSettings", id: "/selected") + XCTAssertEqual(selected, "selected") + let snapshot = try await reopened.snapshot(collections: ["repoSettings"]) + XCTAssertEqual(Set(snapshot.records.keys), ["repoSettings"]) + XCTAssertEqual(reopened.residentCollectionNames, resident) + XCTAssertThrowsError(try reopened.value(String.self, in: "unrelated", id: "large")) + } + + func testUndeclaredTransactionReadFailsWithoutCommittingEarlierWrites() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + do { + try await fixture.store.transaction { transaction in + try transaction.set("must roll back", in: "writes", id: "one") + _ = try transaction.values(String.self, in: "undeclared") + } + XCTFail("A missing read scope must never be treated as an empty collection") + } catch LocalDataError.collectionNotLoaded(let collection) { + XCTAssertEqual(collection, "undeclared") + } + let writes = try await fixture.store.readValues(String.self, in: "writes") + XCTAssertTrue(writes.isEmpty) + } + + func testConcurrentReadModifyWriteTransactionsAreSerialized() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + let tasks = (0..<20).map { _ in + Task { + try await fixture.store.transaction(reading: ["counter"]) { transaction in + let count = try transaction.value(Int.self, in: "counter", id: "value") ?? 0 + try transaction.set(count + 1, in: "counter", id: "value") + } + } + } + for task in tasks { try await task.value } + let value = try await fixture.store.readValue(Int.self, in: "counter", id: "value") + XCTAssertEqual(value, 20) + let reopened = try await fixture.reopen() + let persisted = try await reopened.readValue(Int.self, in: "counter", id: "value") + XCTAssertEqual(persisted, 20) + } + + func testResidentSnapshotUpdatesOnlyAfterSuccessfulCommit() async throws { + let fixture = try LocalDataStoreTestFixture() + defer { fixture.cleanup() } + try await fixture.store.transaction { + try $0.set("account-one", in: "providerPreferences", id: "remote") + } + XCTAssertEqual(try fixture.store.value(String.self, in: "providerPreferences", id: "remote"), "account-one") + try await fixture.store.transaction { $0.remove(in: "providerPreferences", id: "remote") } + XCTAssertNil(try fixture.store.value(String.self, in: "providerPreferences", id: "remote")) + } +} diff --git a/macgitTests/RepoSettingsStoreTests.swift b/macgitTests/RepoSettingsStoreTests.swift index aef651b..72a8da5 100644 --- a/macgitTests/RepoSettingsStoreTests.swift +++ b/macgitTests/RepoSettingsStoreTests.swift @@ -51,8 +51,8 @@ final class RepoSettingsStoreTests: XCTestCase { try await store.update(for: repoA, settings: repoASettings) let freshStore = RepoSettingsStore(dataStore: try await fixture.reopen()) - let loadedA = freshStore.settings(for: repoA, currentBranch: "main", remotes: ["origin"]) - let loadedB = freshStore.settings(for: repoB, currentBranch: nil, remotes: ["upstream"]) + let loadedA = await freshStore.settings(for: repoA, currentBranch: "main", remotes: ["origin"]) + let loadedB = await freshStore.settings(for: repoB, currentBranch: nil, remotes: ["upstream"]) XCTAssertEqual(loadedA, repoASettings) XCTAssertTrue(loadedA.skipProtectedBranchCommitWarnings) diff --git a/macgitTests/RepositoryBookmarkTests.swift b/macgitTests/RepositoryBookmarkTests.swift index e770d55..cf74778 100644 --- a/macgitTests/RepositoryBookmarkTests.swift +++ b/macgitTests/RepositoryBookmarkTests.swift @@ -34,7 +34,7 @@ final class RepositoryBookmarkTests: XCTestCase { try transaction.set(repo.path, in: "bookmarkPaths", id: old.id) } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() await controller.linkMatchingBookmarks(to: [repo]) XCTAssertEqual(controller.bookmarksNeedingAttention(at: repo).map(\.id), [old.id]) let remotes = try await GitStatusService.shared.repositoryBookmarkRemotes(in: repo) @@ -48,9 +48,12 @@ final class RepositoryBookmarkTests: XCTestCase { XCTAssertNil(controller.localURL(for: old)) XCTAssertTrue(controller.bookmarksNeedingAttention(at: repo).isEmpty) let reopened = try await fixture.reopen() - XCTAssertEqual(try reopened.value(RepositoryBookmark.self, in: "bookmarks", id: updated.id), updated) - XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkUploads", id: updated.id)) - XCTAssertNotNil(try reopened.value(String.self, in: "bookmarkDeletes", id: old.id)) + let loadedValue1 = try await reopened.readValue(RepositoryBookmark.self, in: "bookmarks", id: updated.id) + XCTAssertEqual(loadedValue1, updated) + let loadedValue2 = try await reopened.readValue(String.self, in: "bookmarkUploads", id: updated.id) + XCTAssertNotNil(loadedValue2) + let loadedValue3 = try await reopened.readValue(String.self, in: "bookmarkDeletes", id: old.id) + XCTAssertNotNil(loadedValue3) } @MainActor @@ -69,7 +72,7 @@ final class RepositoryBookmarkTests: XCTestCase { } } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() let remotes = try await GitStatusService.shared.repositoryBookmarkRemotes(in: repo) let origin = try XCTUnwrap(remotes.first { $0.name == "origin" }) let updated = try await controller.updateBookmark(old, from: repo, remote: origin) @@ -89,7 +92,7 @@ final class RepositoryBookmarkTests: XCTestCase { let old = try makeBookmark("https://github.com/team/old-client.git") try await fixture.store.transaction { try $0.set(old, in: "bookmarks", id: old.id) } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() let remotes = try await GitStatusService.shared.repositoryBookmarkRemotes(in: repo) let origin = try XCTUnwrap(remotes.first { $0.name == "origin" }) _ = try await GitStatusService.shared.runGit(arguments: ["remote", "set-url", "origin", "https://github.com/other/repo.git"], in: repo) @@ -133,8 +136,10 @@ final class RepositoryBookmarkTests: XCTestCase { XCTAssertEqual(Array(cloud.stored.values), [updated]) XCTAssertEqual(second.bookmarks.map(\.id), [updated.id]) XCTAssertFalse(second.hasPendingChanges) - XCTAssertNil(try reopened.value(String.self, in: "bookmarkUploads", id: updated.id)) - XCTAssertNil(try reopened.value(String.self, in: "bookmarkDeletes", id: old.id)) + let loadedValue4 = try await reopened.readValue(String.self, in: "bookmarkUploads", id: updated.id) + XCTAssertNil(loadedValue4) + let loadedValue5 = try await reopened.readValue(String.self, in: "bookmarkDeletes", id: old.id) + XCTAssertNil(loadedValue5) } @MainActor @@ -147,7 +152,7 @@ final class RepositoryBookmarkTests: XCTestCase { let old = try makeBookmark("https://github.com/team/old-client.git") try await fixture.store.transaction { try $0.set(old, in: "bookmarks", id: old.id) } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() let model = RepositoryBookmarkRepairModel(bookmark: old) await model.selectRepository(repo) XCTAssertEqual(model.selectedRemoteID, "origin") @@ -169,7 +174,7 @@ final class RepositoryBookmarkTests: XCTestCase { let old = try makeBookmark("https://github.com/team/old-client.git") try await fixture.store.transaction { try $0.set(old, in: "bookmarks", id: old.id) } let controller = RepositoryBookmarkController(cloudStore: nil, dataStore: fixture.store) - try controller.load() + try await controller.load() let model = RepositoryBookmarkRepairModel(bookmark: old) await model.selectRepository(repo) await controller.removeBookmark(old) @@ -217,7 +222,7 @@ final class RepositoryBookmarkTests: XCTestCase { try transaction.set(bookmark, in: "bookmarks", id: bookmark.id) } } - try controller.load() + try await controller.load() await controller.linkMatchingBookmarks(to: [repo, repo]) XCTAssertEqual(controller.localURL(for: github), repo) diff --git a/macgitTests/RepositoryCommitRuleSyncControllerTests.swift b/macgitTests/RepositoryCommitRuleSyncControllerTests.swift index 7608beb..a634907 100644 --- a/macgitTests/RepositoryCommitRuleSyncControllerTests.swift +++ b/macgitTests/RepositoryCommitRuleSyncControllerTests.swift @@ -19,7 +19,7 @@ final class RepositoryCommitRuleSyncControllerTests: XCTestCase { let warning = await controller.reconcile(repositoryURL: url, uid: "user-a", cloud: cloud) { applied = $0 } XCTAssertNil(warning) XCTAssertEqual(applied, true) - let loaded = local.settings(for: url.path, currentBranch: nil, remotes: []) + let loaded = await local.settings(for: url.path, currentBranch: nil, remotes: []) XCTAssertTrue(loaded.skipProtectedBranchCommitWarnings) XCTAssertEqual(loaded.userName, "Local author") XCTAssertEqual(cloud.saves, []) @@ -71,8 +71,10 @@ final class RepositoryCommitRuleSyncControllerTests: XCTestCase { } XCTAssertNil(warning) XCTAssertEqual(cloud.saves, [false]) - XCTAssertEqual(local.settings(for: url.path, currentBranch: nil, remotes: []).userName, "Changed while loading") - XCTAssertTrue(try fixture.store.values(Bool.self, in: "commitRulePending").isEmpty) + let current = await local.settings(for: url.path, currentBranch: nil, remotes: []) + XCTAssertEqual(current.userName, "Changed while loading") + let loadedValue1 = try await fixture.store.readValues(Bool.self, in: "commitRulePending") + XCTAssertTrue(loadedValue1.isEmpty) } } diff --git a/macgitTests/RepositoryVisibilityControllerTests.swift b/macgitTests/RepositoryVisibilityControllerTests.swift index d91610b..e851c72 100644 --- a/macgitTests/RepositoryVisibilityControllerTests.swift +++ b/macgitTests/RepositoryVisibilityControllerTests.swift @@ -125,20 +125,13 @@ final class RepositoryVisibilityControllerTests: XCTestCase { let now = Date(timeIntervalSince1970: 1_800_000_000) await cache.save(.unknown, for: repository, resolvedAt: now) - XCTAssertNil(cache.cachedVisibility(for: repository, maximumAge: 900, now: now)) - + let unknown = await cache.cachedVisibility(for: repository, maximumAge: 900, now: now) + XCTAssertNil(unknown) await cache.save(.private, for: repository, resolvedAt: now) - XCTAssertEqual( - cache.cachedVisibility(for: repository, maximumAge: 900, now: now), - .private - ) - XCTAssertNil( - cache.cachedVisibility( - for: repository, - maximumAge: 900, - now: now.addingTimeInterval(901) - ) - ) + let saved = await cache.cachedVisibility(for: repository, maximumAge: 900, now: now) + XCTAssertEqual(saved, .private) + let expired = await cache.cachedVisibility(for: repository, maximumAge: 900, now: now.addingTimeInterval(901)) + XCTAssertNil(expired) } private let repositoryURL = URL(fileURLWithPath: "/tmp/repository") From db2732e332e1db0b854dae146d53013986b37b80 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 06:45:42 +0700 Subject: [PATCH 05/10] perf: coordinate cloud lifecycle once per app --- .../plans/2026-09-27-memory-cold-start.md | 24 +++++-- macgit/App/AppCloudLifecycleController.swift | 64 +++++++++++++++++ macgit/App/FeatureAccessController.swift | 8 +++ macgit/App/macgitApp.swift | 57 +++++++++------ macgit/Views/MainWindow/ContentView.swift | 4 -- .../AppCloudLifecycleControllerTests.swift | 71 +++++++++++++++++++ .../FeatureAccessControllerTests.swift | 15 ++++ 7 files changed, 210 insertions(+), 33 deletions(-) create mode 100644 macgit/App/AppCloudLifecycleController.swift create mode 100644 macgitTests/AppCloudLifecycleControllerTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index 42dd4ad..9ef0729 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -1,6 +1,6 @@ # Cải thiện RAM và cold start -Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown) phần 3 (PR cache) và phần 4 (local storage). Phần 5–6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown), phần 3 (PR cache), phần 4 (local storage) và phần 5 (cloud lifecycle). Phần 6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. @@ -66,12 +66,12 @@ Kiểm tra: DB mới/cũ, migration lỗi giữa chừng, ghi đồng thời, tr Điểm sửa chính: `macgitApp`, `AccountSessionController`, `FeatureAccessController` và các controller sync. -- [ ] Liệt kê dịch vụ/listener bắt đầu trong init và điều kiện thực sự cần chúng. -- [ ] Tách tạo đối tượng khỏi bắt đầu đồng bộ; trì hoãn phần không cần cho first window. -- [ ] Giữ bootstrap bắt buộc trước khi dùng Firebase API. -- [ ] Không làm yếu auth, entitlement, device enforcement hoặc feature policy trong thời gian chờ. -- [ ] Bảo đảm start idempotent; không nhân listener theo window. -- [ ] Hủy listener/task đúng khi đổi session; bỏ kết quả từ session cũ. +- [x] Liệt kê dịch vụ/listener bắt đầu trong init và điều kiện thực sự cần chúng. +- [x] Tách tạo đối tượng khỏi bắt đầu đồng bộ; trì hoãn phần không cần cho first window. +- [x] Giữ bootstrap bắt buộc trước khi dùng Firebase API. +- [x] Không làm yếu auth, entitlement, device enforcement hoặc feature policy trong thời gian chờ. +- [x] Bảo đảm start idempotent; không nhân listener theo window. +- [x] Hủy listener/task đúng khi đổi session; bỏ kết quả từ session cũ. Kiểm tra: guest, session được khôi phục, offline, đăng nhập/đăng xuất, đổi account, nhiều window, policy và entitlement cập nhật. Phân biệt trì hoãn công việc với giảm RAM ổn định. @@ -151,3 +151,13 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Migration vẫn import trong transaction và kiểm tra từng record trước khi đánh dấu hoàn tất; không nạp toàn bộ database để verification. Lỗi đọc pending marker được chuyển tới xử lý lỗi sync để tránh hiểu nhầm là không có thay đổi local. - Thêm coverage cho đọc có phạm vi, không giữ collection không liên quan, rollback khi đọc thiếu scope, ghi đồng thời và cập nhật resident snapshot. - Validation: build macOS **PASS**, **60/60 test PASS** trong các nhóm local storage, migration, bookmark, settings, sync, visibility và credential stores; commit-rule sync sau thay đổi cuối **4/4 test PASS**. Không launch/relaunch app; chưa đo mức giảm RAM/cold start. + +### Phần 5 — Cloud lifecycle (2026-09-27) + +- Phase 4 đã commit tại `33166a1` (`perf: load local data on demand`). +- Inventory khởi động: Firebase bootstrap và khôi phục/claim device vẫn chạy trước khi công bố authenticated session; entitlement và settings sync chỉ bắt đầu sau session hợp lệ. Git Flow và commit-rule cloud store chỉ làm I/O khi repository dùng tính năng tương ứng. AI managed usage vẫn tải theo nhu cầu. +- Feature policy dùng ngay cached/bundled policy nhưng chỉ mở live listener sau khi first window hoàn tất initial setup. `start()` idempotent nên nhiều window không nhân listener. +- Provider-account và bookmark sync được chuyển khỏi từng `ContentView` sang `AppCloudLifecycleController` cấp app. Cùng một session chỉ reconcile một lần; provider và bookmark hydrate song song. Khi session đổi trong lúc đang sync, workflow hoàn tất thao tác shared-store đang chạy, bỏ session trung gian đã lỗi thời và chỉ áp dụng session mới nhất. +- Device observation, entitlement observation và settings observation vẫn giữ generation/UID guard và cleanup hiện có. Bookmark listener tiếp tục bị thay thế khi account đổi; callback cũ bị chặn bằng active UID. +- Coverage phase 5: lifecycle start idempotent, nhiều window cùng session, thay session nhanh, feature listener trì hoãn và chỉ start một lần; regression auth/device, settings sync, provider accounts và bookmarks: **64/64 test PASS**. +- Không launch/relaunch app; chưa kiểm tra tương tác nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 5 chưa commit. diff --git a/macgit/App/AppCloudLifecycleController.swift b/macgit/App/AppCloudLifecycleController.swift new file mode 100644 index 0000000..d4a3c79 --- /dev/null +++ b/macgit/App/AppCloudLifecycleController.swift @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Combine +import Foundation + +/// Starts optional cloud work after the first window is ready and keeps it +/// process-scoped even when several SwiftUI windows request the same session. +@MainActor +final class AppCloudLifecycleController: ObservableObject { + private enum AccountRequest: Equatable { + case notStarted + case session(String?) + } + + private let startFeaturePolicy: () -> Void + private let synchronizeAccount: (AccountSnapshot?) async -> Void + private var didStart = false + private var accountRequest = AccountRequest.notStarted + private var pendingAccount: (request: AccountRequest, account: AccountSnapshot?)? + private var isSynchronizingAccount = false + private var accountWaiters: [CheckedContinuation] = [] + + init( + startFeaturePolicy: @escaping () -> Void, + synchronizeAccount: @escaping (AccountSnapshot?) async -> Void + ) { + self.startFeaturePolicy = startFeaturePolicy + self.synchronizeAccount = synchronizeAccount + } + + func start() { + guard !didStart else { return } + didStart = true + startFeaturePolicy() + } + + func updateAccount(_ account: AccountSnapshot?) async { + let request = AccountRequest.session(account?.uid) + if accountRequest == request { + if isSynchronizingAccount { + await withCheckedContinuation { accountWaiters.append($0) } + } + return + } + + accountRequest = request + pendingAccount = (request, account) + guard !isSynchronizingAccount else { + await withCheckedContinuation { accountWaiters.append($0) } + return + } + + isSynchronizingAccount = true + while let pendingAccount { + self.pendingAccount = nil + // Account reconciliation mutates shared local stores. Complete the + // active session, then skip directly to the newest queued session. + await synchronizeAccount(pendingAccount.account) + } + isSynchronizingAccount = false + let waiters = accountWaiters + accountWaiters.removeAll() + waiters.forEach { $0.resume() } + } +} diff --git a/macgit/App/FeatureAccessController.swift b/macgit/App/FeatureAccessController.swift index 6d35518..dba5afa 100644 --- a/macgit/App/FeatureAccessController.swift +++ b/macgit/App/FeatureAccessController.swift @@ -29,6 +29,7 @@ final class FeatureAccessController: ObservableObject { private let provider: FeaturePolicyProviding? private let cache: FeaturePolicyCaching private var observation: ObservationToken? + private var didStart = false init( provider: FeaturePolicyProviding?, @@ -45,6 +46,13 @@ final class FeatureAccessController: ObservableObject { policy = .bundled } + } + + /// The cached or bundled policy is immediately available. Live policy + /// updates begin once the first app window has finished initial setup. + func start() { + guard !didStart else { return } + didStart = true startObservation() } diff --git a/macgit/App/macgitApp.swift b/macgit/App/macgitApp.swift index 987026b..dc6a1c1 100644 --- a/macgit/App/macgitApp.swift +++ b/macgit/App/macgitApp.swift @@ -31,6 +31,7 @@ struct macgitApp: App { @StateObject private var repositoryVisibilityController: RepositoryVisibilityController @StateObject private var repositoryBookmarkController: RepositoryBookmarkController @StateObject private var gitFlowConfigurationSyncController: GitFlowConfigurationSyncController + @StateObject private var cloudLifecycleController: AppCloudLifecycleController @FocusedValue(\.repositoryWindowCommandState) private var repositoryWindowCommandState init() { @@ -79,23 +80,22 @@ struct macgitApp: App { : nil let providerStore = LocalFirstGitProviderAccountStore(cloudStore: providerCloudStore) let providerTokenVault = KeychainGitProviderTokenVault() - _providerAccountController = StateObject( - wrappedValue: GitProviderAccountController( - store: providerStore, - tokenVault: providerTokenVault, - authService: GitHubProviderAuthService(configuration: providerConfiguration), - configuration: providerConfiguration, - gitLabAuthService: GitLabProviderAuthService(configuration: gitLabProviderConfiguration), - gitLabRedirectURI: gitLabProviderConfiguration.redirectURI, - openURL: NSWorkspace.shared.open, - multipleAccountAccess: { - featureAccessController.decision( - for: .multipleProviderAccounts, - entitlement: accountController.entitlement - ) - } - ) + let providerAccountController = GitProviderAccountController( + store: providerStore, + tokenVault: providerTokenVault, + authService: GitHubProviderAuthService(configuration: providerConfiguration), + configuration: providerConfiguration, + gitLabAuthService: GitLabProviderAuthService(configuration: gitLabProviderConfiguration), + gitLabRedirectURI: gitLabProviderConfiguration.redirectURI, + openURL: NSWorkspace.shared.open, + multipleAccountAccess: { + featureAccessController.decision( + for: .multipleProviderAccounts, + entitlement: accountController.entitlement + ) + } ) + _providerAccountController = StateObject(wrappedValue: providerAccountController) let managedUsage = CommitPlusAIUsageController() managedUsage.setSession(uid: accountController.account?.uid) let managedTokens = FirebaseCommitPlusAITokenProvider() @@ -130,13 +130,12 @@ struct macgitApp: App { cache: SQLiteRepositoryVisibilityCache() ) ) - _repositoryBookmarkController = StateObject( - wrappedValue: RepositoryBookmarkController( - cloudStore: cloudFeaturesEnabled - ? FirestoreRepositoryBookmarkStore() - : nil - ) + let repositoryBookmarkController = RepositoryBookmarkController( + cloudStore: cloudFeaturesEnabled + ? FirestoreRepositoryBookmarkStore() + : nil ) + _repositoryBookmarkController = StateObject(wrappedValue: repositoryBookmarkController) _gitFlowConfigurationSyncController = StateObject( wrappedValue: GitFlowConfigurationSyncController( cloudStore: cloudFeaturesEnabled @@ -144,6 +143,16 @@ struct macgitApp: App { : nil ) ) + _cloudLifecycleController = StateObject( + wrappedValue: AppCloudLifecycleController( + startFeaturePolicy: featureAccessController.start, + synchronizeAccount: { account in + async let providerAccounts: Void = providerAccountController.updateMacgitAccount(account) + async let bookmarks: Void = repositoryBookmarkController.updateAccount(account) + _ = await (providerAccounts, bookmarks) + } + ) + ) } private func performUndoMenuAction(_ action: GitUndoMenuAction) { @@ -228,6 +237,10 @@ struct macgitApp: App { .task { appUpdateController.start() } + .task(id: accountController.account?.uid) { + cloudLifecycleController.start() + await cloudLifecycleController.updateAccount(accountController.account) + } .onChange(of: accountController.account?.uid, initial: true) { _, uid in aiProviderController.managedUsageController?.setSession(uid: uid) } diff --git a/macgit/Views/MainWindow/ContentView.swift b/macgit/Views/MainWindow/ContentView.swift index 1dc2515..9bf0419 100644 --- a/macgit/Views/MainWindow/ContentView.swift +++ b/macgit/Views/MainWindow/ContentView.swift @@ -214,10 +214,6 @@ struct ContentView: View { value: RepositoryWindowRequest.repositoryPicker() ) } - .task(id: accountController.account?.uid) { - await providerAccountController.updateMacgitAccount(accountController.account) - await repositoryBookmarkController.updateAccount(accountController.account) - } .background( RepositoryWindowReader( repositoryWindowContext: windowContext, diff --git a/macgitTests/AppCloudLifecycleControllerTests.swift b/macgitTests/AppCloudLifecycleControllerTests.swift new file mode 100644 index 0000000..c16f08b --- /dev/null +++ b/macgitTests/AppCloudLifecycleControllerTests.swift @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +@MainActor +final class AppCloudLifecycleControllerTests: XCTestCase { + func testStartIsIdempotent() { + var starts = 0 + let controller = AppCloudLifecycleController( + startFeaturePolicy: { starts += 1 }, + synchronizeAccount: { _ in } + ) + + controller.start() + controller.start() + + XCTAssertEqual(starts, 1) + } + + func testSameSessionSynchronizesOnlyOnceAcrossWindows() async { + var synchronizedUIDs: [String] = [] + let controller = AppCloudLifecycleController( + startFeaturePolicy: {}, + synchronizeAccount: { account in + synchronizedUIDs.append(account?.uid ?? "guest") + } + ) + let account = account(uid: "user-one") + + let first = Task { await controller.updateAccount(account) } + await Task.yield() + let second = Task { await controller.updateAccount(account) } + await first.value + await second.value + + XCTAssertEqual(synchronizedUIDs, ["user-one"]) + } + + func testSessionChangesAreAppliedInOrderAndStaleQueuedWorkIsSkipped() async { + var synchronizedUIDs: [String] = [] + var releaseFirst: CheckedContinuation? + let firstStarted = expectation(description: "First session synchronization started") + let controller = AppCloudLifecycleController( + startFeaturePolicy: {}, + synchronizeAccount: { account in + synchronizedUIDs.append(account?.uid ?? "guest") + if account?.uid == "user-one" { + firstStarted.fulfill() + await withCheckedContinuation { releaseFirst = $0 } + } + } + ) + + let first = Task { await controller.updateAccount(account(uid: "user-one")) } + await fulfillment(of: [firstStarted]) + let second = Task { await controller.updateAccount(account(uid: "user-two")) } + await Task.yield() + let guest = Task { await controller.updateAccount(nil) } + await Task.yield() + releaseFirst?.resume() + await first.value + await second.value + await guest.value + + XCTAssertEqual(synchronizedUIDs, ["user-one", "guest"]) + } + + private func account(uid: String) -> AccountSnapshot { + AccountSnapshot(uid: uid, email: nil, displayName: nil, providerIDs: []) + } +} diff --git a/macgitTests/FeatureAccessControllerTests.swift b/macgitTests/FeatureAccessControllerTests.swift index c09a345..77289eb 100644 --- a/macgitTests/FeatureAccessControllerTests.swift +++ b/macgitTests/FeatureAccessControllerTests.swift @@ -47,6 +47,7 @@ final class FeatureAccessControllerTests: XCTestCase { XCTAssertEqual(controller.policyLastUpdatedAt, updatedAt) XCTAssertTrue(controller.isUsingCachedPolicy) + controller.start() provider.fail("Firestore unavailable") XCTAssertEqual(controller.policy, .bundled) @@ -64,6 +65,7 @@ final class FeatureAccessControllerTests: XCTestCase { features: FeatureAccessPolicy.bundled.features ) + controller.start() provider.send(remote) XCTAssertEqual(controller.policy, remote) @@ -71,10 +73,22 @@ final class FeatureAccessControllerTests: XCTestCase { XCTAssertNil(controller.policyError) XCTAssertFalse(controller.isUsingCachedPolicy) } + + func testLiveObservationIsDeferredAndStartsOnlyOnce() { + let provider = FakeFeaturePolicyProvider() + let controller = FeatureAccessController(provider: provider, cache: FakeFeaturePolicyCache()) + + XCTAssertEqual(provider.observeCount, 0) + controller.start() + controller.start() + + XCTAssertEqual(provider.observeCount, 1) + } } @MainActor private final class FakeFeaturePolicyProvider: FeaturePolicyProviding { + private(set) var observeCount = 0 private var onChange: ((FeatureAccessPolicy) -> Void)? private var onError: ((String) -> Void)? @@ -82,6 +96,7 @@ private final class FakeFeaturePolicyProvider: FeaturePolicyProviding { onChange: @escaping (FeatureAccessPolicy) -> Void, onError: @escaping (String) -> Void ) -> ObservationToken { + observeCount += 1 self.onChange = onChange self.onError = onError return FakeFeaturePolicyObservationToken() From be16e24813eef91e1ed1a553a17cac98ed4abdf6 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 07:00:46 +0700 Subject: [PATCH 06/10] perf: Bound caches and release resources for cold start Add access-ordered BoundedMemoryCache for branch list and history snapshots. Limit undo stacks to 50 entries and drop unreferenced file snapshots. Cancel tasks and free trees on window close. --- .../plans/2026-09-27-memory-cold-start.md | 30 ++++++---- macgit/App/RepositoryAIChatController.swift | 11 ++++ macgit/App/RevisionBrowserController.swift | 11 ++++ macgit/Services/BoundedMemoryCache.swift | 49 +++++++++++++++ macgit/Services/BranchListCache.swift | 30 +++++++--- macgit/Services/GitUndoModels.swift | 60 +++++++++++++++++++ macgit/Views/History/HistoryView.swift | 8 +-- .../Views/History/RevisionBrowserView.swift | 1 + macgit/Views/MainWindow/MainWindowView.swift | 1 + macgitTests/BoundedMemoryCacheTests.swift | 20 +++++++ macgitTests/BranchListCacheTests.swift | 19 ++++++ macgitTests/GitUndoManagerTests.swift | 30 ++++++++++ .../RevisionBrowserControllerTests.swift | 22 +++++++ 13 files changed, 270 insertions(+), 22 deletions(-) create mode 100644 macgit/Services/BoundedMemoryCache.swift create mode 100644 macgitTests/BoundedMemoryCacheTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index 9ef0729..cc2c124 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -1,6 +1,6 @@ # Cải thiện RAM và cold start -Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown), phần 3 (PR cache), phần 4 (local storage) và phần 5 (cloud lifecycle). Phần 6 chưa triển khai. Kiểm tra tương tác runtime còn chờ. +Ngày: 2026-09-27. Trạng thái: đã triển khai phần 1 (Welcome), phần 2 (AI, còn mục dropdown), phần 3 (PR cache), phần 4 (local storage), phần 5 (cloud lifecycle) và phần 6 (cache/lifecycle còn lại). Kiểm tra tương tác runtime còn chờ. Không có bước đo baseline, theo yêu cầu của người dùng. Triển khai từng phần độc lập để dễ review và kiểm tra. Không đặt mục tiêu giảm MB hoặc thời gian cụ thể khi chưa có phép đo. Không tự launch/relaunch app. @@ -77,14 +77,14 @@ Kiểm tra: guest, session được khôi phục, offline, đăng nhập/đăng ## 6. Cache và vòng đời còn lại -- [ ] History: giới hạn snapshot theo branch/filter; giữ selection và viewport khi background refresh. -- [ ] Branch/reference cache: giới hạn entry và invalidate sau mutation. -- [ ] Diff, preview, ảnh và thumbnail: giới hạn kích thước/số entry, tải theo nhu cầu và hủy khi đóng. -- [ ] Welcome activity: kiểm tra payload commit hash, giới hạn entry và giải phóng dữ liệu không hiển thị. -- [ ] AI chat: kiểm tra conversation/context/tool output được giữ lại và vòng đời controller. -- [ ] Undo: kiểm tra giới hạn/vòng đời nhưng không xóa dữ liệu cần cho undo còn hiệu lực. -- [ ] Window/controller: kiểm tra task, timer, observer, closure và subscription giữ đối tượng sau khi đóng. -- [ ] Ghi nhận từng mục đã có giới hạn hợp lý; không refactor chỉ để thay đổi kiến trúc. +- [x] History: giới hạn snapshot theo branch/filter; giữ selection và viewport khi background refresh. +- [x] Branch/reference cache: giới hạn entry và invalidate sau mutation. +- [x] Diff, preview, ảnh và thumbnail: giới hạn kích thước/số entry, tải theo nhu cầu và hủy khi đóng. +- [x] Welcome activity: kiểm tra payload commit hash, giới hạn entry và giải phóng dữ liệu không hiển thị. +- [x] AI chat: kiểm tra conversation/context/tool output được giữ lại và vòng đời controller. +- [x] Undo: kiểm tra giới hạn/vòng đời nhưng không xóa dữ liệu cần cho undo còn hiệu lực. +- [x] Window/controller: kiểm tra task, timer, observer, closure và subscription giữ đối tượng sau khi đóng. +- [x] Ghi nhận từng mục đã có giới hạn hợp lý; không refactor chỉ để thay đổi kiến trúc. Kiểm tra: mở/đóng nhiều repo, chuyển branch/filter, mở file lớn, chuyển PR, chat dài và undo/redo. Mỗi cache phải có owner, giới hạn, invalidation và điểm giải phóng rõ ràng. @@ -160,4 +160,14 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Provider-account và bookmark sync được chuyển khỏi từng `ContentView` sang `AppCloudLifecycleController` cấp app. Cùng một session chỉ reconcile một lần; provider và bookmark hydrate song song. Khi session đổi trong lúc đang sync, workflow hoàn tất thao tác shared-store đang chạy, bỏ session trung gian đã lỗi thời và chỉ áp dụng session mới nhất. - Device observation, entitlement observation và settings observation vẫn giữ generation/UID guard và cleanup hiện có. Bookmark listener tiếp tục bị thay thế khi account đổi; callback cũ bị chặn bằng active UID. - Coverage phase 5: lifecycle start idempotent, nhiều window cùng session, thay session nhanh, feature listener trì hoãn và chỉ start một lần; regression auth/device, settings sync, provider accounts và bookmarks: **64/64 test PASS**. -- Không launch/relaunch app; chưa kiểm tra tương tác nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 5 chưa commit. +- Không launch/relaunch app; chưa kiểm tra tương tác nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 5 đã commit tại `db2732e3`. + +### Phần 6 — Cache và vòng đời còn lại (2026-09-28) + +- Phase 5 đã commit tại `db2732e3` (`perf: coordinate cloud lifecycle once per app`). +- Thêm `BoundedMemoryCache` dùng access-order. Cache branch/reference giới hạn 32 entry, History giữ tối đa 3 snapshot branch/filter; entry cũ bị loại và request đang chạy bị hủy khi invalidate. +- Undo/redo giữ tối đa 50 action. Entry bị loại, redo bị thay thế và thao tác clear đều xóa file snapshot không còn được stack nào tham chiếu, nên dữ liệu phục vụ undo còn hiệu lực vẫn được giữ. +- Revision Browser hủy task và giải phóng tree/preview khi đóng. Repository AI hủy request/timer, pending operation và dữ liệu selector tạm khi window đóng. +- Các cache còn lại đã được audit và giữ nguyên khi đã có owner/giới hạn phù hợp: Welcome activity 20 entry, syntax-highlight preview 512 dòng không dài, revision tree 50.000 entry, PR payload dùng SQLite có giới hạn, diff/image/video theo vòng đời view, AI history lưu SQLite và chỉ conversation hiện tại resident. +- Coverage cache/History/undo/revision: **48/48 test PASS**. Regression Repository AI agent/remote lifecycle: **19/19 test PASS**. Build macOS: **PASS**; `git diff --check`: **PASS**. +- Không launch/relaunch app; chưa kiểm tra tương tác mở/đóng nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 6 chưa commit. diff --git a/macgit/App/RepositoryAIChatController.swift b/macgit/App/RepositoryAIChatController.swift index 004a456..3bb580d 100644 --- a/macgit/App/RepositoryAIChatController.swift +++ b/macgit/App/RepositoryAIChatController.swift @@ -379,6 +379,17 @@ final class RepositoryAIChatController: ObservableObject { activeRequestTask.cancel() } + func windowWillClose() { + activeRequestTask?.cancel() + pendingMutationExpirationTask?.cancel() + pendingMutationExpirationTask = nil + pendingRemoteOperationExpirationTask?.cancel() + pendingRemoteOperationExpirationTask = nil + pendingMutation = nil + pendingRemoteOperation = nil + dismissSelection() + } + func confirmPendingMutation(id: UUID) async { guard !isExecutingMutation, let pending = pendingMutation, diff --git a/macgit/App/RevisionBrowserController.swift b/macgit/App/RevisionBrowserController.swift index 1ea5787..ed0ec76 100644 --- a/macgit/App/RevisionBrowserController.swift +++ b/macgit/App/RevisionBrowserController.swift @@ -144,4 +144,15 @@ final class RevisionBrowserController { isLoading = false isLoadingPreview = false } + + func releaseResources() { + cancel() + snapshot = nil + children.removeAll() + expanded.removeAll() + folderErrors.removeAll() + selectedEntry = nil + preview = nil + previewError = nil + } } diff --git a/macgit/Services/BoundedMemoryCache.swift b/macgit/Services/BoundedMemoryCache.swift new file mode 100644 index 0000000..4995f33 --- /dev/null +++ b/macgit/Services/BoundedMemoryCache.swift @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import Foundation + +/// Small access-ordered cache for state that must not grow with every +/// repository, branch, filter, or search visited in one app session. +nonisolated struct BoundedMemoryCache { + let capacity: Int + private var values: [Key: Value] = [:] + private var accessOrder: [Key] = [] + + init(capacity: Int) { + precondition(capacity > 0) + self.capacity = capacity + } + + var count: Int { values.count } + var keys: Dictionary.Keys { values.keys } + + mutating func value(for key: Key) -> Value? { + guard let value = values[key] else { return nil } + touch(key) + return value + } + + @discardableResult + mutating func insert(_ value: Value, for key: Key) -> (key: Key, value: Value)? { + values[key] = value + touch(key) + guard values.count > capacity, let evictedKey = accessOrder.first else { return nil } + accessOrder.removeFirst() + return values.removeValue(forKey: evictedKey).map { (evictedKey, $0) } + } + + @discardableResult + mutating func removeValue(forKey key: Key) -> Value? { + accessOrder.removeAll { $0 == key } + return values.removeValue(forKey: key) + } + + mutating func removeAll() { + values.removeAll() + accessOrder.removeAll() + } + + private mutating func touch(_ key: Key) { + accessOrder.removeAll { $0 == key } + accessOrder.append(key) + } +} diff --git a/macgit/Services/BranchListCache.swift b/macgit/Services/BranchListCache.swift index 4bc91b0..bfd8374 100644 --- a/macgit/Services/BranchListCache.swift +++ b/macgit/Services/BranchListCache.swift @@ -19,6 +19,7 @@ import Foundation actor BranchListCache { static let ttl: TimeInterval = 120 + static let defaultCapacity = 32 enum Key: Hashable { case local(URL) @@ -42,16 +43,20 @@ actor BranchListCache { let task: Task<[String], Never> } - private var entries: [Key: Entry] = [:] + private var entries: BoundedMemoryCache private var generations: [Key: Int] = [:] private var inFlight: [Key: InFlight] = [:] + init(capacity: Int = BranchListCache.defaultCapacity) { + entries = BoundedMemoryCache(capacity: capacity) + } + func values( for key: Key, now: Date = Date(), load: @escaping @Sendable () async -> [String] ) async -> [String] { - if let entry = entries[key], now.timeIntervalSince(entry.createdAt) < Self.ttl { + if let entry = entries.value(for: key), now.timeIntervalSince(entry.createdAt) < Self.ttl { return entry.values } @@ -64,12 +69,17 @@ actor BranchListCache { inFlight[key] = InFlight(generation: generation, task: task) let values = await task.value - if generations[key, default: 0] == generation { - entries[key] = Entry(values: values, createdAt: now) - } if inFlight[key]?.generation == generation { inFlight[key] = nil } + if generations[key, default: 0] == generation { + if let evicted = entries.insert(Entry(values: values, createdAt: now), for: key), + inFlight[evicted.key] == nil { + generations[evicted.key] = nil + } + } else if inFlight[key] == nil && entries.keys.contains(key) == false { + generations[key] = nil + } return values } @@ -105,9 +115,13 @@ actor BranchListCache { } private func invalidate(_ key: Key) { - entries[key] = nil - generations[key, default: 0] += 1 - inFlight[key] = nil + entries.removeValue(forKey: key) + if let request = inFlight.removeValue(forKey: key) { + generations[key, default: 0] += 1 + request.task.cancel() + } else { + generations[key] = nil + } } private func isRemoteKey(_ key: Key) -> Bool { diff --git a/macgit/Services/GitUndoModels.swift b/macgit/Services/GitUndoModels.swift index cc11d5a..0804469 100644 --- a/macgit/Services/GitUndoModels.swift +++ b/macgit/Services/GitUndoModels.swift @@ -83,6 +83,19 @@ indirect enum GitUndoOperation: Equatable { case recreateGitFlowWorktree(path: URL, branch: String, baseTip: String, label: String?) } +private extension GitUndoOperation { + var fileSnapshotIDs: Set { + switch self { + case .restoreFileSnapshot(let id), .deleteFileSnapshot(let id): + [id] + case .sequence(let operations): + operations.reduce(into: Set()) { $0.formUnion($1.fileSnapshotIDs) } + default: + [] + } + } +} + @MainActor final class OpenRepositoryRegistry { static let shared = OpenRepositoryRegistry() @@ -207,8 +220,20 @@ enum GitUndoEntryFactory { @MainActor final class GitUndoManager: ObservableObject { + nonisolated static let defaultStackLimit = 50 @Published private(set) var undoStack: [GitUndoEntry] = [] @Published private(set) var redoStack: [GitUndoEntry] = [] + private let stackLimit: Int + private let discardEntry: (GitUndoEntry) -> Void + + init( + stackLimit: Int = GitUndoManager.defaultStackLimit, + discardEntry: ((GitUndoEntry) -> Void)? = nil + ) { + precondition(stackLimit > 0) + self.stackLimit = stackLimit + self.discardEntry = discardEntry ?? Self.deleteFileSnapshots + } var canUndo: Bool { !undoStack.isEmpty @@ -229,8 +254,11 @@ final class GitUndoManager: ObservableObject { } func register(_ entry: GitUndoEntry) { + let discardedRedo = redoStack undoStack.append(entry) redoStack.removeAll() + trimUndoStack() + discard(discardedRedo) } func popForUndo() -> GitUndoEntry? { @@ -251,6 +279,7 @@ final class GitUndoManager: ObservableObject { func completeRedo(_ entry: GitUndoEntry) { undoStack.append(entry) + trimUndoStack() } func restoreRedo(_ entry: GitUndoEntry) { @@ -258,7 +287,38 @@ final class GitUndoManager: ObservableObject { } func removeAll() { + let discarded = undoStack + redoStack undoStack.removeAll() redoStack.removeAll() + discard(discarded) + } + + private func trimUndoStack() { + guard undoStack.count > stackLimit else { return } + let discarded = Array(undoStack.prefix(undoStack.count - stackLimit)) + undoStack.removeFirst(discarded.count) + discard(discarded) + } + + private func discard(_ entries: [GitUndoEntry]) { + let retainedSnapshotIDs = (undoStack + redoStack).reduce(into: Set()) { + $0.formUnion($1.undoOperation.fileSnapshotIDs) + $0.formUnion($1.redoOperation.fileSnapshotIDs) + } + for entry in entries { + let snapshotIDs = entry.undoOperation.fileSnapshotIDs + .union(entry.redoOperation.fileSnapshotIDs) + guard snapshotIDs.isDisjoint(with: retainedSnapshotIDs) else { continue } + discardEntry(entry) + } + } + + private static func deleteFileSnapshots(in entry: GitUndoEntry) { + let store = GitFileUndoSnapshotStore() + let snapshotIDs = entry.undoOperation.fileSnapshotIDs + .union(entry.redoOperation.fileSnapshotIDs) + for id in snapshotIDs { + try? store.delete(snapshotID: id, in: entry.repositoryURL) + } } } diff --git a/macgit/Views/History/HistoryView.swift b/macgit/Views/History/HistoryView.swift index 7125465..0123668 100644 --- a/macgit/Views/History/HistoryView.swift +++ b/macgit/Views/History/HistoryView.swift @@ -80,7 +80,7 @@ struct HistoryView: View { @State private var showingError = false @State private var scrollTarget: String? = nil @State private var paging = HistoryPagingState(pageSize: 120) - @State private var historyCache: [String: HistorySnapshot] = [:] + @State private var historyCache = BoundedMemoryCache(capacity: 3) @State private var historySearchText = "" @State private var debouncedHistorySearchText = "" @State private var historySearchDebounceTask: Task? = nil @@ -1111,7 +1111,7 @@ struct HistoryView: View { preservingSelectionAndScroll: Bool = false ) async { let cacheKey = historyLoadKey - if reset, let cached = historyCache[cacheKey] { + if reset, let cached = historyCache.value(for: cacheKey) { await MainActor.run { applyCachedSnapshot(cached) } @@ -1305,11 +1305,11 @@ struct HistoryView: View { } cancelHistoryRefreshIndicator() - historyCache[cacheKey] = HistorySnapshot( + historyCache.insert(HistorySnapshot( commits: loadedCommits, graphModel: newGraphModel, selectedCommitHash: selectedCommit?.hash - ) + ), for: cacheKey) // Appending a page also updates the native Table's rows and can // transiently clear its selection, just like a background refresh. diff --git a/macgit/Views/History/RevisionBrowserView.swift b/macgit/Views/History/RevisionBrowserView.swift index ea14313..779851b 100644 --- a/macgit/Views/History/RevisionBrowserView.swift +++ b/macgit/Views/History/RevisionBrowserView.swift @@ -39,6 +39,7 @@ struct RevisionBrowserView: View { } } .task { controller.load() } + .onDisappear { controller.releaseResources() } } private var tree: some View { diff --git a/macgit/Views/MainWindow/MainWindowView.swift b/macgit/Views/MainWindow/MainWindowView.swift index 5849591..7a1b6ff 100644 --- a/macgit/Views/MainWindow/MainWindowView.swift +++ b/macgit/Views/MainWindow/MainWindowView.swift @@ -751,6 +751,7 @@ struct MainWindowView: View { reason: "Repository window closed.", appendTranscript: false ) + repositoryAIChatController.windowWillClose() OpenRepositoryRegistry.shared.unregister(repositoryURL) syncState.stopBackgroundSync() } diff --git a/macgitTests/BoundedMemoryCacheTests.swift b/macgitTests/BoundedMemoryCacheTests.swift new file mode 100644 index 0000000..10f09d4 --- /dev/null +++ b/macgitTests/BoundedMemoryCacheTests.swift @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +import XCTest +@testable import macgit + +final class BoundedMemoryCacheTests: XCTestCase { + func testRecentlyReadValueSurvivesEviction() { + var cache = BoundedMemoryCache(capacity: 2) + cache.insert(1, for: "one") + cache.insert(2, for: "two") + + XCTAssertEqual(cache.value(for: "one"), 1) + let evicted = cache.insert(3, for: "three") + + XCTAssertEqual(evicted?.key, "two") + XCTAssertNil(cache.value(for: "two")) + XCTAssertEqual(cache.value(for: "one"), 1) + XCTAssertEqual(cache.value(for: "three"), 3) + XCTAssertEqual(cache.count, 2) + } +} diff --git a/macgitTests/BranchListCacheTests.swift b/macgitTests/BranchListCacheTests.swift index 33f469d..1115525 100644 --- a/macgitTests/BranchListCacheTests.swift +++ b/macgitTests/BranchListCacheTests.swift @@ -145,6 +145,25 @@ final class BranchListCacheTests: XCTestCase { XCTAssertEqual(results.1, ["main"]) XCTAssertEqual(callCount, 1) } + + func testLeastRecentlyUsedEntryIsEvictedAtCapacity() async { + let cache = BranchListCache(capacity: 2) + let now = Date(timeIntervalSince1970: 0) + let first = URL(fileURLWithPath: "/tmp/repo-first") + let second = URL(fileURLWithPath: "/tmp/repo-second") + let third = URL(fileURLWithPath: "/tmp/repo-third") + + _ = await cache.values(for: .local(first), now: now) { ["first"] } + _ = await cache.values(for: .local(second), now: now) { ["second"] } + _ = await cache.values(for: .local(first), now: now) { ["unexpected"] } + _ = await cache.values(for: .local(third), now: now) { ["third"] } + + let retainedFirst = await cache.values(for: .local(first), now: now) { ["unexpected"] } + let reloadedSecond = await cache.values(for: .local(second), now: now) { ["reloaded"] } + + XCTAssertEqual(reloadedSecond, ["reloaded"]) + XCTAssertEqual(retainedFirst, ["first"]) + } } private actor CallCounter { diff --git a/macgitTests/GitUndoManagerTests.swift b/macgitTests/GitUndoManagerTests.swift index 5478b88..133a6bd 100644 --- a/macgitTests/GitUndoManagerTests.swift +++ b/macgitTests/GitUndoManagerTests.swift @@ -146,6 +146,36 @@ final class GitUndoManagerTests: XCTestCase { XCTAssertEqual(entry.redoOperation, .commit(message: "ship it", noVerify: true, signOff: true)) } + func testUndoStackEvictsOldestEntryAndDiscardsItsResources() { + var discarded: [GitUndoEntry] = [] + let manager = GitUndoManager(stackLimit: 2) { discarded.append($0) } + let first = entry(label: "First") + let second = entry(label: "Second") + let third = entry(label: "Third") + + manager.register(first) + manager.register(second) + manager.register(third) + + XCTAssertEqual(manager.undoStack, [second, third]) + XCTAssertEqual(discarded, [first]) + } + + func testRegisterDiscardsRedoResourcesAndRemoveAllDiscardsRemainingEntries() { + var discarded: [GitUndoEntry] = [] + let manager = GitUndoManager(discardEntry: { discarded.append($0) }) + let first = entry(label: "First") + let second = entry(label: "Second") + + manager.register(first) + _ = manager.popForUndo() + manager.completeUndo(first) + manager.register(second) + manager.removeAll() + + XCTAssertEqual(discarded, [first, second]) + } + private func entry(label: String) -> GitUndoEntry { GitUndoEntry( id: UUID(uuidString: "00000000-0000-0000-0000-000000000001")!, diff --git a/macgitTests/RevisionBrowserControllerTests.swift b/macgitTests/RevisionBrowserControllerTests.swift index 4a124bc..76191bc 100644 --- a/macgitTests/RevisionBrowserControllerTests.swift +++ b/macgitTests/RevisionBrowserControllerTests.swift @@ -58,6 +58,28 @@ final class RevisionBrowserControllerTests: XCTestCase { let cachedCalls = await service.treeCalls XCTAssertEqual(cachedCalls, calls) } + + func testReleaseResourcesClearsTreeAndPreviewPayloads() async throws { + let service = BrowserTestService() + let controller = RevisionBrowserController( + repositoryURL: URL(fileURLWithPath: "/tmp/browser"), + revision: "HEAD", + service: service + ) + controller.load() + await controller.loadTask?.value + let entry = try XCTUnwrap(controller.children[""]?[1]) + controller.select(entry) + await controller.previewTask?.value + + controller.releaseResources() + + XCTAssertNil(controller.snapshot) + XCTAssertTrue(controller.children.isEmpty) + XCTAssertTrue(controller.expanded.isEmpty) + XCTAssertNil(controller.selectedEntry) + XCTAssertNil(controller.preview) + } } private actor BrowserTestService: RevisionBrowserServing { From bc285ffb825133bf9d9b51f5c386a92de195d356 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 07:12:40 +0700 Subject: [PATCH 07/10] perf: Close Welcome window while repository windows are open Track app-wide repository window count and reopen Welcome when the last window closes. --- .../plans/2026-09-27-memory-cold-start.md | 3 +- .../RepositoryWindowLifecycleController.swift | 22 +++++++++ macgit/App/macgitApp.swift | 4 +- macgit/Views/MainWindow/ContentView.swift | 25 ++++++++++- ...sitoryWindowLifecycleControllerTests.swift | 45 +++++++++++++++++++ 5 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 macgit/App/RepositoryWindowLifecycleController.swift create mode 100644 macgitTests/RepositoryWindowLifecycleControllerTests.swift diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index cc2c124..5336304 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -168,6 +168,7 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Thêm `BoundedMemoryCache` dùng access-order. Cache branch/reference giới hạn 32 entry, History giữ tối đa 3 snapshot branch/filter; entry cũ bị loại và request đang chạy bị hủy khi invalidate. - Undo/redo giữ tối đa 50 action. Entry bị loại, redo bị thay thế và thao tác clear đều xóa file snapshot không còn được stack nào tham chiếu, nên dữ liệu phục vụ undo còn hiệu lực vẫn được giữ. - Revision Browser hủy task và giải phóng tree/preview khi đóng. Repository AI hủy request/timer, pending operation và dữ liệu selector tạm khi window đóng. +- Welcome được đóng khi một repository xuất hiện. Main window được đếm ở cấp app; khi main window cuối cùng đóng, Welcome được mở lại, tránh giữ đồng thời hai view tree trong luồng sử dụng repository thông thường. - Các cache còn lại đã được audit và giữ nguyên khi đã có owner/giới hạn phù hợp: Welcome activity 20 entry, syntax-highlight preview 512 dòng không dài, revision tree 50.000 entry, PR payload dùng SQLite có giới hạn, diff/image/video theo vòng đời view, AI history lưu SQLite và chỉ conversation hiện tại resident. -- Coverage cache/History/undo/revision: **48/48 test PASS**. Regression Repository AI agent/remote lifecycle: **19/19 test PASS**. Build macOS: **PASS**; `git diff --check`: **PASS**. +- Coverage cache/History/undo/revision: **48/48 test PASS**. Regression Repository AI agent/remote lifecycle: **19/19 test PASS**. Window lifecycle: **4/4 test PASS**. Build macOS: **PASS**; `git diff --check`: **PASS**. - Không launch/relaunch app; chưa kiểm tra tương tác mở/đóng nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 6 chưa commit. diff --git a/macgit/App/RepositoryWindowLifecycleController.swift b/macgit/App/RepositoryWindowLifecycleController.swift new file mode 100644 index 0000000..59ae466 --- /dev/null +++ b/macgit/App/RepositoryWindowLifecycleController.swift @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import Foundation + +@MainActor +final class RepositoryWindowLifecycleController { + private var activeWindowIDs: Set = [] + + @discardableResult + func windowDidAppear(id: UUID) -> Bool { + activeWindowIDs.insert(id).inserted + } + + func windowDidDisappear(id: UUID) -> Bool { + guard activeWindowIDs.remove(id) != nil else { return false } + return activeWindowIDs.isEmpty + } + + var activeWindowCount: Int { + activeWindowIDs.count + } +} diff --git a/macgit/App/macgitApp.swift b/macgit/App/macgitApp.swift index dc6a1c1..a37b33b 100644 --- a/macgit/App/macgitApp.swift +++ b/macgit/App/macgitApp.swift @@ -32,6 +32,7 @@ struct macgitApp: App { @StateObject private var repositoryBookmarkController: RepositoryBookmarkController @StateObject private var gitFlowConfigurationSyncController: GitFlowConfigurationSyncController @StateObject private var cloudLifecycleController: AppCloudLifecycleController + private let repositoryWindowLifecycleController = RepositoryWindowLifecycleController() @FocusedValue(\.repositoryWindowCommandState) private var repositoryWindowCommandState init() { @@ -225,7 +226,8 @@ struct macgitApp: App { isWelcomeWindow: isWelcomeWindow, accountController: accountController, providerAccountController: providerAccountController, - aiProviderController: aiProviderController + aiProviderController: aiProviderController, + repositoryWindowLifecycleController: repositoryWindowLifecycleController ) .environmentObject(appState) .environmentObject(appUpdateController) diff --git a/macgit/Views/MainWindow/ContentView.swift b/macgit/Views/MainWindow/ContentView.swift index 9bf0419..ce398d6 100644 --- a/macgit/Views/MainWindow/ContentView.swift +++ b/macgit/Views/MainWindow/ContentView.swift @@ -25,9 +25,11 @@ struct ContentView: View { @EnvironmentObject private var featureAccessController: FeatureAccessController @EnvironmentObject private var repositoryBookmarkController: RepositoryBookmarkController @Environment(\.openWindow) private var openWindow + @Environment(\.dismissWindow) private var dismissWindow @ObservedObject var accountController: AccountSessionController @ObservedObject var providerAccountController: GitProviderAccountController @ObservedObject var aiProviderController: AIProviderController + let repositoryWindowLifecycleController: RepositoryWindowLifecycleController let isWelcomeWindow: Bool let initialShowsHistory: Bool? @@ -45,6 +47,7 @@ struct ContentView: View { @State private var shouldFitScreenWhenRepositoryOpens = false @State private var webOpeningProgressID: UUID? @State private var windowContext = RepositoryWindowContext() + @State private var windowLifecycleID = UUID() @StateObject private var operationProgress = RepositoryOperationProgress() init( @@ -52,13 +55,15 @@ struct ContentView: View { isWelcomeWindow: Bool = false, accountController: AccountSessionController, providerAccountController: GitProviderAccountController, - aiProviderController: AIProviderController + aiProviderController: AIProviderController, + repositoryWindowLifecycleController: RepositoryWindowLifecycleController ) { self.initialShowsHistory = request?.showsHistory self.isWelcomeWindow = isWelcomeWindow self.accountController = accountController self.providerAccountController = providerAccountController self.aiProviderController = aiProviderController + self.repositoryWindowLifecycleController = repositoryWindowLifecycleController _repositoryURL = State(initialValue: request?.repositoryURL) _showingCloneSheet = State( initialValue: request?.initialPresentation == .cloneRepository @@ -100,6 +105,19 @@ struct ContentView: View { } } .onOpenURL(perform: handleExternalURL) + .onAppear { + guard !isWelcomeWindow else { return } + repositoryWindowLifecycleController.windowDidAppear(id: windowLifecycleID) + closeWelcomeIfRepositoryIsOpen() + } + .onChange(of: repositoryURL) { _, _ in + closeWelcomeIfRepositoryIsOpen() + } + .onDisappear { + guard !isWelcomeWindow, + repositoryWindowLifecycleController.windowDidDisappear(id: windowLifecycleID) else { return } + openWindow(id: "welcome") + } .alert("Cannot Open Repository", isPresented: $showingRepositoryOpenError) { } message: { Text(repositoryOpenError) @@ -255,6 +273,11 @@ struct ContentView: View { } } + private func closeWelcomeIfRepositoryIsOpen() { + guard !isWelcomeWindow, repositoryURL != nil else { return } + dismissWindow(id: "welcome") + } + private var accountSheetPresentation: Binding { Binding( get: { diff --git a/macgitTests/RepositoryWindowLifecycleControllerTests.swift b/macgitTests/RepositoryWindowLifecycleControllerTests.swift new file mode 100644 index 0000000..7fc4033 --- /dev/null +++ b/macgitTests/RepositoryWindowLifecycleControllerTests.swift @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import XCTest +@testable import macgit + +@MainActor +final class RepositoryWindowLifecycleControllerTests: XCTestCase { + func testFirstAppearanceRegistersWindow() { + let controller = RepositoryWindowLifecycleController() + let id = UUID() + + XCTAssertTrue(controller.windowDidAppear(id: id)) + XCTAssertEqual(controller.activeWindowCount, 1) + } + + func testRepeatedAppearanceDoesNotDuplicateWindow() { + let controller = RepositoryWindowLifecycleController() + let id = UUID() + + XCTAssertTrue(controller.windowDidAppear(id: id)) + XCTAssertFalse(controller.windowDidAppear(id: id)) + XCTAssertEqual(controller.activeWindowCount, 1) + } + + func testClosingOneOfMultipleWindowsDoesNotRestoreWelcome() { + let controller = RepositoryWindowLifecycleController() + let first = UUID() + let second = UUID() + controller.windowDidAppear(id: first) + controller.windowDidAppear(id: second) + + XCTAssertFalse(controller.windowDidDisappear(id: first)) + XCTAssertEqual(controller.activeWindowCount, 1) + } + + func testClosingLastWindowRestoresWelcome() { + let controller = RepositoryWindowLifecycleController() + let id = UUID() + controller.windowDidAppear(id: id) + + XCTAssertTrue(controller.windowDidDisappear(id: id)) + XCTAssertEqual(controller.activeWindowCount, 0) + XCTAssertFalse(controller.windowDidDisappear(id: id)) + } +} From 13038f1d42378dea547aa8c479effeb8c34eb449 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 07:26:00 +0700 Subject: [PATCH 08/10] perf: Defer repository window screen fit until welcome closes --- .../plans/2026-09-27-memory-cold-start.md | 3 +- macgit/App/AppState.swift | 10 ++++++ macgit/App/macgitApp.swift | 2 +- macgit/Views/Common/GeneralSettingsView.swift | 13 ++++++- macgit/Views/MainWindow/ContentView.swift | 4 +-- macgit/Views/MainWindow/WelcomeView.swift | 5 ++- .../WindowInitialScreenFitModifier.swift | 34 +++++++++++++------ macgitTests/AppSettingsSnapshotTests.swift | 15 ++++++++ 8 files changed, 70 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/plans/2026-09-27-memory-cold-start.md b/docs/superpowers/plans/2026-09-27-memory-cold-start.md index 5336304..d6b7487 100644 --- a/docs/superpowers/plans/2026-09-27-memory-cold-start.md +++ b/docs/superpowers/plans/2026-09-27-memory-cold-start.md @@ -169,6 +169,7 @@ Không tự commit, push hoặc thay đổi release. Các checkbox chỉ đượ - Undo/redo giữ tối đa 50 action. Entry bị loại, redo bị thay thế và thao tác clear đều xóa file snapshot không còn được stack nào tham chiếu, nên dữ liệu phục vụ undo còn hiệu lực vẫn được giữ. - Revision Browser hủy task và giải phóng tree/preview khi đóng. Repository AI hủy request/timer, pending operation và dữ liệu selector tạm khi window đóng. - Welcome được đóng khi một repository xuất hiện. Main window được đếm ở cấp app; khi main window cuối cùng đóng, Welcome được mở lại, tránh giữ đồng thời hai view tree trong luồng sử dụng repository thông thường. +- Repository window mặc định được chủ động đặt về cùng kích thước `1180×780` với Welcome, thay vì để macOS khôi phục frame full-size cũ. General Settings có toggle local theo máy để người dùng chọn fit repository window mới vào toàn bộ vùng màn hình; mặc định tắt. - Các cache còn lại đã được audit và giữ nguyên khi đã có owner/giới hạn phù hợp: Welcome activity 20 entry, syntax-highlight preview 512 dòng không dài, revision tree 50.000 entry, PR payload dùng SQLite có giới hạn, diff/image/video theo vòng đời view, AI history lưu SQLite và chỉ conversation hiện tại resident. -- Coverage cache/History/undo/revision: **48/48 test PASS**. Regression Repository AI agent/remote lifecycle: **19/19 test PASS**. Window lifecycle: **4/4 test PASS**. Build macOS: **PASS**; `git diff --check`: **PASS**. +- Coverage cache/History/undo/revision: **48/48 test PASS**. Regression Repository AI agent/remote lifecycle: **19/19 test PASS**. Settings/request/window lifecycle: **23/23 test PASS**. Build macOS: **PASS**; `git diff --check`: **PASS**. - Không launch/relaunch app; chưa kiểm tra tương tác mở/đóng nhiều window bằng runtime và chưa đo mức giảm RAM/cold start. Phần 6 chưa commit. diff --git a/macgit/App/AppState.swift b/macgit/App/AppState.swift index 751bc7a..9b6e5c5 100644 --- a/macgit/App/AppState.swift +++ b/macgit/App/AppState.swift @@ -38,6 +38,7 @@ final class AppState: ObservableObject { private static let historyIncludeRemotesKey = "historyIncludeRemotes" private static let autoFetchEnabledKey = "autoFetchEnabled" private static let refreshOnAppActiveKey = "refreshOnAppActive" + private static let fitRepositoryWindowsToScreenKey = "fitRepositoryWindowsToScreen" private static let settingsSyncEnabledKey = "settingsSyncEnabled" private static let searchFilterKey = "searchFilter" private static let preferredSearchFileApplicationKey = "preferredSearchFileApplication" @@ -191,6 +192,11 @@ final class AppState: ObservableObject { } } } + @Published var fitRepositoryWindowsToScreen: Bool { + didSet { + userDefaults.set(fitRepositoryWindowsToScreen, forKey: Self.fitRepositoryWindowsToScreenKey) + } + } @Published var syncEnabled: Bool { didSet { userDefaults.set(syncEnabled, forKey: Self.settingsSyncEnabledKey) @@ -242,6 +248,9 @@ final class AppState: ObservableObject { let historyIncludeRemotes = userDefaults.object(forKey: Self.historyIncludeRemotesKey) as? Bool ?? false let autoFetchEnabled = userDefaults.object(forKey: Self.autoFetchEnabledKey) as? Bool ?? false let refreshOnAppActive = userDefaults.object(forKey: Self.refreshOnAppActiveKey) as? Bool ?? true + let fitRepositoryWindowsToScreen = userDefaults.object( + forKey: Self.fitRepositoryWindowsToScreenKey + ) as? Bool ?? false let syncEnabled = userDefaults.object(forKey: Self.settingsSyncEnabledKey) as? Bool ?? false let searchFilter = userDefaults.string(forKey: Self.searchFilterKey) .flatMap(SearchFilter.init(rawValue:)) ?? .all @@ -287,6 +296,7 @@ final class AppState: ObservableObject { self.historyIncludeRemotes = historyIncludeRemotes self.autoFetchEnabled = autoFetchEnabled self.refreshOnAppActive = refreshOnAppActive + self.fitRepositoryWindowsToScreen = fitRepositoryWindowsToScreen self.syncEnabled = syncEnabled self.searchFilter = searchFilter self.preferredSearchFileApplicationBundleIdentifier = preferredSearchFileApplicationBundleIdentifier diff --git a/macgit/App/macgitApp.swift b/macgit/App/macgitApp.swift index a37b33b..9d9f06d 100644 --- a/macgit/App/macgitApp.swift +++ b/macgit/App/macgitApp.swift @@ -270,7 +270,7 @@ struct macgitApp: App { WindowGroup(id: "main", for: RepositoryWindowRequest.self) { request in windowContent(request: request.wrappedValue) } - .defaultSize(width: 860, height: 680) + .defaultSize(width: 1180, height: 780) .defaultLaunchBehavior(.suppressed) .commands { RepositoryFileCommands() diff --git a/macgit/Views/Common/GeneralSettingsView.swift b/macgit/Views/Common/GeneralSettingsView.swift index f226603..08ea2dc 100644 --- a/macgit/Views/Common/GeneralSettingsView.swift +++ b/macgit/Views/Common/GeneralSettingsView.swift @@ -23,6 +23,16 @@ struct GeneralSettingsView: View { var body: some View { Form { + Section { + SettingsToggleRow( + title: "Fill screen when opening repositories", + detail: "Expand newly opened repository windows to the available screen area. When off, they open at the same size as Welcome.", + isOn: $appState.fitRepositoryWindowsToScreen + ) + } header: { + Label("Windows", systemImage: "macwindow") + } + Section { SettingsToggleRow( title: "Show Git Flow", @@ -86,7 +96,7 @@ struct GeneralSettingsView: View { Button("Restore Defaults", role: .destructive, action: restoreDefaults) Button("Cancel", role: .cancel) {} } message: { - Text("Sidebar, History, and Pull & Fetch preferences on this page will be reset.") + Text("Window, Sidebar, History, and Pull & Fetch preferences on this page will be reset.") } } @@ -95,6 +105,7 @@ struct GeneralSettingsView: View { } private func restoreDefaults() { + appState.fitRepositoryWindowsToScreen = false appState.showGitFlow = true appState.showSubmodules = false appState.showSubtrees = false diff --git a/macgit/Views/MainWindow/ContentView.swift b/macgit/Views/MainWindow/ContentView.swift index 8702302..072d516 100644 --- a/macgit/Views/MainWindow/ContentView.swift +++ b/macgit/Views/MainWindow/ContentView.swift @@ -407,11 +407,11 @@ struct ContentView: View { id: "main", value: RepositoryWindowRequest.repository( url, - shouldFitVisibleScreen: true + shouldFitVisibleScreen: appState.fitRepositoryWindowsToScreen ) ) } else { - shouldFitScreenWhenRepositoryOpens = true + shouldFitScreenWhenRepositoryOpens = appState.fitRepositoryWindowsToScreen repositoryURL = url } } diff --git a/macgit/Views/MainWindow/WelcomeView.swift b/macgit/Views/MainWindow/WelcomeView.swift index 4335424..4d01e69 100644 --- a/macgit/Views/MainWindow/WelcomeView.swift +++ b/macgit/Views/MainWindow/WelcomeView.swift @@ -20,6 +20,7 @@ import SwiftUI struct WelcomeView: View { @Environment(\.openWindow) private var openWindow @EnvironmentObject private var bookmarkController: RepositoryBookmarkController + @EnvironmentObject private var appState: AppState @State private var locationError: String? @ObservedObject private var store = RecentRepositoriesStore.shared @State private var model = WelcomeDashboardModel() @@ -85,7 +86,9 @@ struct WelcomeView: View { } store.add(repository.url) openWindow(id: "main", value: RepositoryWindowRequest.repository( - repository.url, shouldFitVisibleScreen: true, showsHistory: repository.showsHistory + repository.url, + shouldFitVisibleScreen: appState.fitRepositoryWindowsToScreen, + showsHistory: repository.showsHistory )) } diff --git a/macgit/Views/MainWindow/WindowInitialScreenFitModifier.swift b/macgit/Views/MainWindow/WindowInitialScreenFitModifier.swift index 07b1e59..4da9ccf 100644 --- a/macgit/Views/MainWindow/WindowInitialScreenFitModifier.swift +++ b/macgit/Views/MainWindow/WindowInitialScreenFitModifier.swift @@ -18,48 +18,62 @@ import SwiftUI struct WindowInitialScreenFitModifier: NSViewRepresentable { + static let defaultContentSize = NSSize(width: 1180, height: 780) let isEnabled: Bool func makeNSView(context: Context) -> NSView { let view = NSView() view.isHidden = true - scheduleScreenFit(for: view, coordinator: context.coordinator) + scheduleInitialSize(for: view, coordinator: context.coordinator) return view } func updateNSView(_ nsView: NSView, context: Context) { context.coordinator.isEnabled = isEnabled - scheduleScreenFit(for: nsView, coordinator: context.coordinator) + scheduleInitialSize(for: nsView, coordinator: context.coordinator) } func makeCoordinator() -> Coordinator { Coordinator(isEnabled: isEnabled) } - private func scheduleScreenFit(for view: NSView, coordinator: Coordinator) { + private func scheduleInitialSize(for view: NSView, coordinator: Coordinator) { DispatchQueue.main.async { - coordinator.fitToVisibleScreenIfNeeded(window: view.window) + coordinator.applyInitialSizeIfNeeded(window: view.window) } } final class Coordinator { var isEnabled: Bool - private var didFitWindow = false + private var didApplyInitialSize = false init(isEnabled: Bool) { self.isEnabled = isEnabled } - func fitToVisibleScreenIfNeeded(window: NSWindow?) { - guard isEnabled, - !didFitWindow, + func applyInitialSizeIfNeeded(window: NSWindow?) { + guard !didApplyInitialSize, let window, let screen = window.screen ?? NSScreen.main else { return } - didFitWindow = true - window.setFrame(screen.visibleFrame, display: true) + didApplyInitialSize = true + if isEnabled { + window.setFrame(screen.visibleFrame, display: true) + return + } + + let availableContentSize = window.contentRect(forFrameRect: screen.visibleFrame).size + let contentSize = NSSize( + width: min(WindowInitialScreenFitModifier.defaultContentSize.width, availableContentSize.width), + height: min(WindowInitialScreenFitModifier.defaultContentSize.height, availableContentSize.height) + ) + window.setContentSize(contentSize) + window.setFrameOrigin(NSPoint( + x: screen.visibleFrame.midX - window.frame.width / 2, + y: screen.visibleFrame.midY - window.frame.height / 2 + )) } } } diff --git a/macgitTests/AppSettingsSnapshotTests.swift b/macgitTests/AppSettingsSnapshotTests.swift index 6d94dbb..84b71e1 100644 --- a/macgitTests/AppSettingsSnapshotTests.swift +++ b/macgitTests/AppSettingsSnapshotTests.swift @@ -383,4 +383,19 @@ final class AppSettingsSnapshotTests: XCTestCase { XCTAssertEqual(emissions.count, 2) XCTAssertEqual(emissions.last, expected) } + + func testRepositoryWindowScreenFitDefaultsOffAndPersistsLocally() { + let suiteName = "AppSettingsSnapshotTests.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + let state = AppState(userDefaults: defaults) + XCTAssertFalse(state.fitRepositoryWindowsToScreen) + let cloudSnapshot = state.snapshot + + state.fitRepositoryWindowsToScreen = true + + XCTAssertTrue(AppState(userDefaults: defaults).fitRepositoryWindowsToScreen) + XCTAssertEqual(state.snapshot, cloudSnapshot) + } } From 8b538e3d612e0fe8cdcdd45212fa7443246317fa Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 07:33:08 +0700 Subject: [PATCH 09/10] fix: Align toolbar labels to center instead of text baseline --- macgit/Views/Common/BadgeToolbarButton.swift | 3 +++ macgit/Views/Common/ToolbarButton.swift | 3 +++ 2 files changed, 6 insertions(+) diff --git a/macgit/Views/Common/BadgeToolbarButton.swift b/macgit/Views/Common/BadgeToolbarButton.swift index 0d94037..5e3bea3 100644 --- a/macgit/Views/Common/BadgeToolbarButton.swift +++ b/macgit/Views/Common/BadgeToolbarButton.swift @@ -62,6 +62,9 @@ struct BadgeToolbarButton: View { .frame(maxWidth: .infinity, maxHeight: .infinity) } } + // Keep badge text from changing the toolbar label's alignment. + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] } + .alignmentGuide(.lastTextBaseline) { $0[VerticalAlignment.center] } } .help(label) .disabled(disabled || isLoading) diff --git a/macgit/Views/Common/ToolbarButton.swift b/macgit/Views/Common/ToolbarButton.swift index 2e1580b..935c6d9 100644 --- a/macgit/Views/Common/ToolbarButton.swift +++ b/macgit/Views/Common/ToolbarButton.swift @@ -51,6 +51,9 @@ func toolbarButton(icon: String, label: String, showText: Bool = true, isLoading .scaleEffect(0.6) } } + // Center the complete custom label, rather than an icon/text baseline. + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] } + .alignmentGuide(.lastTextBaseline) { $0[VerticalAlignment.center] } } .help(label) .disabled(disabled || isLoading) From 737e4121e69137cd792a4022ab0da03603a4de4e Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 28 Sep 2026 08:25:58 +0700 Subject: [PATCH 10/10] fix: address cache, undo cleanup, and SQLite review findings --- macgit/Services/BranchListCache.swift | 24 +++++---------- macgit/Services/GitUndoModels.swift | 11 +++---- macgit/Services/LocalSQLiteDatabase.swift | 6 ++-- macgitTests/BranchListCacheTests.swift | 37 +++++++++++++++++++++++ macgitTests/GitUndoManagerTests.swift | 25 +++++++++++++-- macgitTests/LocalDataOnDemandTests.swift | 20 ++++++++++++ 6 files changed, 94 insertions(+), 29 deletions(-) diff --git a/macgit/Services/BranchListCache.swift b/macgit/Services/BranchListCache.swift index bfd8374..14cef69 100644 --- a/macgit/Services/BranchListCache.swift +++ b/macgit/Services/BranchListCache.swift @@ -39,12 +39,11 @@ actor BranchListCache { } private struct InFlight { - let generation: Int + let id: UUID let task: Task<[String], Never> } private var entries: BoundedMemoryCache - private var generations: [Key: Int] = [:] private var inFlight: [Key: InFlight] = [:] init(capacity: Int = BranchListCache.defaultCapacity) { @@ -60,25 +59,19 @@ actor BranchListCache { return entry.values } - let generation = generations[key, default: 0] - if let request = inFlight[key], request.generation == generation { + if let request = inFlight[key] { return await request.task.value } + let id = UUID() let task = Task { await load() } - inFlight[key] = InFlight(generation: generation, task: task) + inFlight[key] = InFlight(id: id, task: task) let values = await task.value - if inFlight[key]?.generation == generation { + // Only the current loader may publish; cancelled loaders can still finish. + if inFlight[key]?.id == id { inFlight[key] = nil - } - if generations[key, default: 0] == generation { - if let evicted = entries.insert(Entry(values: values, createdAt: now), for: key), - inFlight[evicted.key] == nil { - generations[evicted.key] = nil - } - } else if inFlight[key] == nil && entries.keys.contains(key) == false { - generations[key] = nil + entries.insert(Entry(values: values, createdAt: now), for: key) } return values } @@ -117,10 +110,7 @@ actor BranchListCache { private func invalidate(_ key: Key) { entries.removeValue(forKey: key) if let request = inFlight.removeValue(forKey: key) { - generations[key, default: 0] += 1 request.task.cancel() - } else { - generations[key] = nil } } diff --git a/macgit/Services/GitUndoModels.swift b/macgit/Services/GitUndoModels.swift index 0804469..9ef86da 100644 --- a/macgit/Services/GitUndoModels.swift +++ b/macgit/Services/GitUndoModels.swift @@ -224,11 +224,11 @@ final class GitUndoManager: ObservableObject { @Published private(set) var undoStack: [GitUndoEntry] = [] @Published private(set) var redoStack: [GitUndoEntry] = [] private let stackLimit: Int - private let discardEntry: (GitUndoEntry) -> Void + private let discardEntry: (GitUndoEntry, Set) -> Void init( stackLimit: Int = GitUndoManager.defaultStackLimit, - discardEntry: ((GitUndoEntry) -> Void)? = nil + discardEntry: ((GitUndoEntry, Set) -> Void)? = nil ) { precondition(stackLimit > 0) self.stackLimit = stackLimit @@ -308,15 +308,12 @@ final class GitUndoManager: ObservableObject { for entry in entries { let snapshotIDs = entry.undoOperation.fileSnapshotIDs .union(entry.redoOperation.fileSnapshotIDs) - guard snapshotIDs.isDisjoint(with: retainedSnapshotIDs) else { continue } - discardEntry(entry) + discardEntry(entry, snapshotIDs.subtracting(retainedSnapshotIDs)) } } - private static func deleteFileSnapshots(in entry: GitUndoEntry) { + private static func deleteFileSnapshots(in entry: GitUndoEntry, snapshotIDs: Set) { let store = GitFileUndoSnapshotStore() - let snapshotIDs = entry.undoOperation.fileSnapshotIDs - .union(entry.redoOperation.fileSnapshotIDs) for id in snapshotIDs { try? store.delete(snapshotID: id, in: entry.repositoryURL) } diff --git a/macgit/Services/LocalSQLiteDatabase.swift b/macgit/Services/LocalSQLiteDatabase.swift index dbfb672..c49aa2e 100644 --- a/macgit/Services/LocalSQLiteDatabase.swift +++ b/macgit/Services/LocalSQLiteDatabase.swift @@ -68,7 +68,7 @@ actor LocalSQLiteDatabase { guard !collections.isEmpty else { return [:] } return try withDatabase { db in var result: [String: [String: Data]] = [:] - try transaction(db) { + try transaction(db, readOnly: true) { for collection in collections { result[collection] = [:] try query(db, "SELECT id, payload FROM records WHERE collection = ?", [collection]) { row in @@ -88,8 +88,8 @@ actor LocalSQLiteDatabase { return result } - private func transaction(_ db: OpaquePointer, _ operation: () throws -> Void) throws { - try query(db, "BEGIN IMMEDIATE") + private func transaction(_ db: OpaquePointer, readOnly: Bool = false, _ operation: () throws -> Void) throws { + try query(db, readOnly ? "BEGIN DEFERRED" : "BEGIN IMMEDIATE") do { try operation() try query(db, "COMMIT") diff --git a/macgitTests/BranchListCacheTests.swift b/macgitTests/BranchListCacheTests.swift index 1115525..0968ef6 100644 --- a/macgitTests/BranchListCacheTests.swift +++ b/macgitTests/BranchListCacheTests.swift @@ -146,6 +146,26 @@ final class BranchListCacheTests: XCTestCase { XCTAssertEqual(callCount, 1) } + func testCancelledLoaderCannotRepopulateAfterReplacementIsEvicted() async { + let cache = BranchListCache(capacity: 1) + let repository = URL(fileURLWithPath: "/tmp/repo-old") + let gate = BranchLoaderGate() + let old = Task { + await cache.values(for: .local(repository)) { + await gate.wait() + return ["stale"] + } + } + await gate.waitUntilStarted() + await cache.invalidate(repositoryURL: repository) + _ = await cache.values(for: .local(repository)) { ["replacement"] } + _ = await cache.values(for: .local(URL(fileURLWithPath: "/tmp/repo-other"))) { ["other"] } + await gate.release() + _ = await old.value + let result = await cache.values(for: .local(repository)) { ["fresh"] } + XCTAssertEqual(result, ["fresh"]) + } + func testLeastRecentlyUsedEntryIsEvictedAtCapacity() async { let cache = BranchListCache(capacity: 2) let now = Date(timeIntervalSince1970: 0) @@ -166,6 +186,23 @@ final class BranchListCacheTests: XCTestCase { } } +private actor BranchLoaderGate { + private var continuation: CheckedContinuation? + + func wait() async { + await withCheckedContinuation { continuation = $0 } + } + + func waitUntilStarted() async { + while continuation == nil { await Task.yield() } + } + + func release() { + continuation?.resume() + continuation = nil + } +} + private actor CallCounter { private(set) var value = 0 diff --git a/macgitTests/GitUndoManagerTests.swift b/macgitTests/GitUndoManagerTests.swift index 133a6bd..c0e5d86 100644 --- a/macgitTests/GitUndoManagerTests.swift +++ b/macgitTests/GitUndoManagerTests.swift @@ -148,7 +148,7 @@ final class GitUndoManagerTests: XCTestCase { func testUndoStackEvictsOldestEntryAndDiscardsItsResources() { var discarded: [GitUndoEntry] = [] - let manager = GitUndoManager(stackLimit: 2) { discarded.append($0) } + let manager = GitUndoManager(stackLimit: 2) { entry, _ in discarded.append(entry) } let first = entry(label: "First") let second = entry(label: "Second") let third = entry(label: "Third") @@ -163,7 +163,7 @@ final class GitUndoManagerTests: XCTestCase { func testRegisterDiscardsRedoResourcesAndRemoveAllDiscardsRemainingEntries() { var discarded: [GitUndoEntry] = [] - let manager = GitUndoManager(discardEntry: { discarded.append($0) }) + let manager = GitUndoManager(discardEntry: { entry, _ in discarded.append(entry) }) let first = entry(label: "First") let second = entry(label: "Second") @@ -176,6 +176,27 @@ final class GitUndoManagerTests: XCTestCase { XCTAssertEqual(discarded, [first, second]) } + func testEvictionDeletesOnlyUnsharedSnapshots() { + let shared = UUID() + let unshared = UUID() + var deleted: Set = [] + let manager = GitUndoManager(stackLimit: 1) { _, ids in deleted.formUnion(ids) } + let repository = URL(fileURLWithPath: "/tmp/repo") + manager.register(GitUndoEntry( + repositoryURL: repository, label: "Old", + undoOperation: .sequence([.restoreFileSnapshot(id: shared), .restoreFileSnapshot(id: unshared)]), + redoOperation: .deleteFileSnapshot(id: unshared) + )) + manager.register(GitUndoEntry( + repositoryURL: repository, label: "Retained", + undoOperation: .restoreFileSnapshot(id: shared), + redoOperation: .deleteFileSnapshot(id: shared) + )) + XCTAssertEqual(deleted, [unshared]) + manager.removeAll() + XCTAssertEqual(deleted, [shared, unshared]) + } + private func entry(label: String) -> GitUndoEntry { GitUndoEntry( id: UUID(uuidString: "00000000-0000-0000-0000-000000000001")!, diff --git a/macgitTests/LocalDataOnDemandTests.swift b/macgitTests/LocalDataOnDemandTests.swift index e24b481..c52d005 100644 --- a/macgitTests/LocalDataOnDemandTests.swift +++ b/macgitTests/LocalDataOnDemandTests.swift @@ -1,9 +1,29 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import XCTest +import SQLite3 @testable import macgit @MainActor final class LocalDataOnDemandTests: XCTestCase { + func testCollectionSnapshotReadsWhileAnotherConnectionReservesWriter() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + defer { try? FileManager.default.removeItem(at: directory) } + let url = directory.appendingPathComponent("data.sqlite") + let database = LocalSQLiteDatabase(url: url) + let payload = Data("committed".utf8) + try await database.commit([("settings", "one", payload)]) + + var writer: OpaquePointer? + XCTAssertEqual(sqlite3_open(url.path, &writer), SQLITE_OK) + let connection = try XCTUnwrap(writer) + defer { sqlite3_close(connection) } + XCTAssertEqual(sqlite3_exec(connection, "BEGIN IMMEDIATE", nil, nil, nil), SQLITE_OK) + defer { sqlite3_exec(connection, "ROLLBACK", nil, nil, nil) } + + let snapshot = try await database.read(collections: ["settings"]) + XCTAssertEqual(snapshot["settings"]?["one"], payload) + } + func testPrepareAndOnDemandReadsDoNotRetainUnrelatedPayloads() async throws { let fixture = try LocalDataStoreTestFixture() defer { fixture.cleanup() }