diff --git a/oAI/Models/SyncModels.swift b/oAI/Models/SyncModels.swift index 96c1124..e097d2e 100644 --- a/oAI/Models/SyncModels.swift +++ b/oAI/Models/SyncModels.swift @@ -39,6 +39,7 @@ enum SyncError: LocalizedError { case repoNotCloned case secretsDetected([String]) case parseError(String) + case syncInProgress var errorDescription: String? { switch self { @@ -56,6 +57,8 @@ enum SyncError: LocalizedError { return "Secrets detected in conversations: \(secrets.joined(separator: ", ")). Remove before syncing." case .parseError(let message): return "Failed to parse conversation: \(message)" + case .syncInProgress: + return "A sync is already in progress. Try again in a moment." } } } diff --git a/oAI/Services/GitSyncService.swift b/oAI/Services/GitSyncService.swift index ba29cd1..3f9075b 100644 --- a/oAI/Services/GitSyncService.swift +++ b/oAI/Services/GitSyncService.swift @@ -630,6 +630,20 @@ class GitSyncService { return } + // Guard against racing autoSync()/syncNow() on the same working tree — this method and + // autoSync() are both fired from independent, uncoordinated Tasks (this one at app launch, + // autoSync() debounced off chat activity), so without this a pull here could run while + // autoSync() is mid-export, leaving a freshly-written untracked file (folders.json, + // notes.json) that the pull then refuses to merge over: "untracked working tree files + // would be overwritten by merge." Skipping outright (not waiting) is fine here since + // startup sync is a one-time best-effort fetch, not something the user is blocked on. + guard !isSyncing else { + log.debug("Skipping startup sync (another sync already in progress)") + return + } + isSyncing = true + defer { isSyncing = false } + log.info("Running startup sync (pull + import)...") do { @@ -668,6 +682,12 @@ class GitSyncService { // Schedule new sync with 5 second delay pendingSyncTask = Task { + // Tracks whether *this* task is the one holding isSyncing, so the catch block below + // only ever releases a lock it actually acquired — without this, a cancellation while + // still waiting in the loop below (i.e. before this task owns the lock at all) would + // incorrectly clear isSyncing out from under whichever other sync is still running. + var acquiredLock = false + do { // Wait for debounce period try await Task.sleep(for: .seconds(5)) @@ -675,11 +695,23 @@ class GitSyncService { // Check if cancelled during sleep guard !Task.isCancelled else { return } + // Wait for any other sync (startup pull, manual Sync Now) already in flight to + // finish rather than racing it on the same working tree — see syncOnStartup()'s + // guard for what goes wrong otherwise. Waiting (not skipping) here, since + // auto-sync is how local changes actually reach the remote; silently dropping this + // round could leave a push pending indefinitely if nothing else triggers autoSync + // again soon. + while await MainActor.run(body: { isSyncing }) { + guard !Task.isCancelled else { return } + try await Task.sleep(for: .milliseconds(500)) + } + // Set syncing state await MainActor.run { isSyncing = true lastSyncError = nil } + acquiredLock = true log.info("Auto-sync starting (export + push)...") @@ -694,14 +726,17 @@ class GitSyncService { isSyncing = false syncStatus.lastSyncTime = Date() } + acquiredLock = false log.info("Auto-sync completed successfully") } catch { - // Error - await MainActor.run { - isSyncing = false - lastSyncError = error.localizedDescription + // Error — only release the lock if this task actually acquired it + if acquiredLock { + await MainActor.run { + isSyncing = false + lastSyncError = error.localizedDescription + } } log.error("Auto-sync failed: \(error.localizedDescription)") @@ -712,6 +747,27 @@ class GitSyncService { await pendingSyncTask?.value } + /// Manual full sync (the Settings → Sync "Sync Now" button): pull → import → export → push, + /// in that order so the working tree is fully merged before Confab writes its own files back + /// out (see exportAllConversations's ordering note). Throws `.syncInProgress` rather than + /// racing autoSync()/syncOnStartup() if either is already running on the same working tree — + /// same class of bug as the "untracked working tree files" failure those two guard against. + func syncNow() async throws -> (imported: Int, skipped: Int) { + guard !isSyncing else { + throw SyncError.syncInProgress + } + isSyncing = true + defer { isSyncing = false } + + try await pull() + let result = try await importAllConversations() + try await exportAllConversations() + try await push() + + await updateStatus() + return (result.imported, result.skipped) + } + // MARK: - Secret Scanning /// Scan for API keys and secrets in conversations diff --git a/oAI/Views/Screens/SettingsView.swift b/oAI/Views/Screens/SettingsView.swift index 5532cee..55dcda0 100644 --- a/oAI/Views/Screens/SettingsView.swift +++ b/oAI/Views/Screens/SettingsView.swift @@ -3039,31 +3039,13 @@ It's better to admit "I need more information" or "I cannot do that" than to fak private func syncNow() async { isSyncing = true - syncTestResult = nil + syncTestResult = "Syncing..." do { - // Step 1: Pull from remote first, into a working tree that hasn't been touched by - // this round's export yet. Exporting before pulling can write files (e.g. - // folders.json, which every machine writes to the same path) that git then refuses - // to merge over: "untracked working tree files would be overwritten by merge". - syncTestResult = "Pulling changes..." - try await gitSync.pull() - - // Step 2: Import any new/updated conversations and folders from what was just pulled - syncTestResult = "Importing conversations..." - let result = try await gitSync.importAllConversations() - - // Step 3: Export — now safe, re-derives working tree files from the local DB state, - // which already reflects whatever was just imported plus this machine's own changes - syncTestResult = "Exporting conversations..." - try await gitSync.exportAllConversations() - - // Step 4: Push to remote - syncTestResult = "Pushing changes..." - try await gitSync.push() - - // Success - await gitSync.updateStatus() + // Orchestration (pull → import → export → push) and the guard against racing + // autoSync()/syncOnStartup() on the same working tree both live in GitSyncService now + // — see its syncNow() for why. + let result = try await gitSync.syncNow() syncTestResult = "✓ Sync complete: \(result.imported) imported, \(result.skipped) skipped" } catch { syncTestResult = "✗ Sync failed: \(error.localizedDescription)"