From 897b685c70d0a352da8ffdae763cec7c0f736552 Mon Sep 17 00:00:00 2001 From: Amir Khodak Date: Mon, 25 May 2026 15:13:43 +0000 Subject: [PATCH] Fix perpetual sync loop and wrong delete decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs fixed: 1. catchupScan used raw dir.walk() with no filters, causing hidden/excluded files to appear as "new" every startup and trigger a catchup sync. Fixed by using LocalAccessor.walkFiles(pair) which applies the same filters and uses the same mtime source (SAF cursor) as SyncEngine. 2. catchupScan compared localModifiedAt.toEpochMilli() vs File.lastModified() (millisecond precision) while SyncEngine uses second precision. Every file appeared "modified" after a successful sync. Fixed by using epochSecond. 3. syncDecide() treated !localExists && remoteExists && known==null as "user deleted local copy → delete remote" even on files that were never synced. Fixed to treat unknown remote files as new (download them), which is safe because a genuinely-deleted file will always have a known state record from the previous sync. Co-Authored-By: Claude Sonnet 4.6 --- ...otlin-compiler-7270483765496797749.salive} | 0 .../com/syncflow/domain/sync/SyncEngine.kt | 22 ++++++++----------- .../com/syncflow/worker/FileWatchService.kt | 20 +++++++++++------ version.properties | 4 ++-- 4 files changed, 24 insertions(+), 22 deletions(-) rename .kotlin/sessions/{kotlin-compiler-5606529168844077151.salive => kotlin-compiler-7270483765496797749.salive} (100%) diff --git a/.kotlin/sessions/kotlin-compiler-5606529168844077151.salive b/.kotlin/sessions/kotlin-compiler-7270483765496797749.salive similarity index 100% rename from .kotlin/sessions/kotlin-compiler-5606529168844077151.salive rename to .kotlin/sessions/kotlin-compiler-7270483765496797749.salive diff --git a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt index 0f9c128..3fb69bf 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt @@ -146,13 +146,15 @@ class SyncEngine @Inject constructor( storeLocalMtime = false)) } SyncDecision.DELETE_LOCAL -> { - accessor.delete(rel) + val deleted = accessor.delete(rel) + if (!deleted) Timber.w("SyncEngine: DELETE_LOCAL failed (silent) for $rel") fileStateDao.delete(pair.id, rel) logEvent(pair.id, SyncEventType.FILE_DELETED, rel, "local", 0) FileOutcome(deleted = 1) } SyncDecision.DELETE_REMOTE -> { - provider.deleteFile("${pair.remotePath}/$rel") + runCatching { provider.deleteFile("${pair.remotePath}/$rel") } + .onFailure { e -> Timber.e(e, "SyncEngine: DELETE_REMOTE failed for $rel") } fileStateDao.delete(pair.id, rel) logEvent(pair.id, SyncEventType.FILE_DELETED, rel, "remote", 0) FileOutcome(deleted = 1) @@ -283,21 +285,15 @@ internal fun syncDecide( } !localExists && remoteExists -> when { - known == null -> if (!hasPriorSyncState) { - // Initial sync: no history at all — remote files are new, download them. + known == null -> { + // No state record: could be a new remote file OR a file whose state was lost. + // Downloading is always safer than deleting — if the user deleted the local + // copy intentionally, the state record will still exist (known != null) and + // the else-branch below correctly deletes the remote copy. when (direction) { SyncDirection.DOWNLOAD_ONLY, SyncDirection.TWO_WAY -> SyncDecision.DOWNLOAD else -> SyncDecision.SKIP } - } else { - // Pair has been synced before but this file has no state record - // (e.g. uploaded before state-tracking was fixed). Treat the same - // as a known remote-deletion: apply mirror/keep behavior. - when { - deleteBehavior == DeleteBehavior.KEEP -> SyncDecision.SKIP - direction == SyncDirection.UPLOAD_ONLY || direction == SyncDirection.TWO_WAY -> SyncDecision.DELETE_REMOTE - else -> SyncDecision.SKIP - } } else -> when { deleteBehavior == DeleteBehavior.KEEP -> SyncDecision.SKIP diff --git a/app/src/main/kotlin/com/syncflow/worker/FileWatchService.kt b/app/src/main/kotlin/com/syncflow/worker/FileWatchService.kt index c3e5d93..fafdea8 100644 --- a/app/src/main/kotlin/com/syncflow/worker/FileWatchService.kt +++ b/app/src/main/kotlin/com/syncflow/worker/FileWatchService.kt @@ -20,7 +20,9 @@ import com.syncflow.MainActivity import com.syncflow.R import com.syncflow.data.db.SyncFileStateDao import com.syncflow.data.db.SyncPairDao +import com.syncflow.data.db.entities.toDomain import com.syncflow.domain.model.ScheduleType +import com.syncflow.domain.sync.LocalAccessor import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.* import kotlinx.coroutines.sync.Mutex @@ -224,21 +226,25 @@ class FileWatchService : Service() { val known = fileStateDao.getForPair(pairId).associateBy { it.relativePath } if (known.isEmpty()) return // Never synced — first sync will be triggered manually - val current = mutableMapOf() - dir.walk().filter { it.isFile }.forEach { f -> - current[f.relativeTo(dir).path.replace('\\', '/')] = f.lastModified() - } + val pairEntity = syncPairDao.getById(pairId) ?: return + val pair = pairEntity.toDomain() + // Use the same accessor + filters as SyncEngine so hidden/excluded/size-filtered files + // don't appear as "new" in the catchup scan and trigger a perpetual sync loop. + val accessor = if (pair.localPath.startsWith("content://")) + LocalAccessor.Saf(Uri.parse(pair.localPath), contentResolver) + else + LocalAccessor.JavaFile(dir) + val current = accessor.walkFiles(pair) val hasNew = current.any { (rel, _) -> rel !in known } - val hasModified = current.any { (rel, mtime) -> + val hasModified = current.any { (rel, info) -> val s = known[rel]; s != null && s.localModifiedAt != null && - s.localModifiedAt.toEpochMilli() != mtime + s.localModifiedAt.epochSecond != info.lastModifiedMs / 1000 } val hasDeleted = known.keys.any { rel -> rel !in current } if (hasNew || hasModified || hasDeleted) { Timber.d("FileWatchService: catchup detected changes for pair $pairId, scheduling sync") - val pair = syncPairDao.getById(pairId) ?: return // Cancel any debounce that started before our startup cooldown was set debounceJobs[pairId]?.cancel() debounceJobs.remove(pairId) diff --git a/version.properties b/version.properties index b2a8657..6da62ed 100644 --- a/version.properties +++ b/version.properties @@ -1,2 +1,2 @@ -VERSION_NAME=1.0.32 -VERSION_CODE=33 +VERSION_NAME=1.0.37 +VERSION_CODE=38