Fix perpetual sync loop and wrong delete decisions

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 <noreply@anthropic.com>
v1.0.32: fix manual sync loop via WorkManager tag monitor
2026-05-25 15:13:43 +00:00 · 2026-05-25 14:11:58 +00:00 · 2026-05-25 14:06:19 +00:00 · 2026-05-25 13:48:18 +00:00
6 changed files with 163 additions and 97 deletions
@@ -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
@@ -20,9 +20,13 @@ 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
+import kotlinx.coroutines.sync.withLock
 import timber.log.Timber
 import java.io.File
 import javax.inject.Inject
@@ -35,13 +39,17 @@ class FileWatchService : Service() {

    private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
    private val mainHandler = Handler(Looper.getMainLooper())
+    // Prevents concurrent refresh() calls from doubling watchers + catchup scans
+    private val refreshMutex = Mutex()

    // Multiple FileObserver instances per pair: one per directory (recursive)
    private val fileObservers = mutableMapOf<Long, MutableList<FileObserver>>()
    private val contentObservers = mutableMapOf<Long, ContentObserver>()
    private val debounceJobs = mutableMapOf<Long, Job>()
-    // After a watcher-triggered sync completes, suppress FileObserver events for this long
-    // to stop the feedback loop: sync writes files → FileObserver fires → another sync → repeat.
+    // Persistent monitors that watch WorkManager for ANY sync (manual, catchup, onchange)
+    // so the cooldown is set regardless of who triggered the sync.
+    private val syncMonitorJobs = mutableMapOf<Long, Job>()
+    // After a sync completes, suppress FileObserver events for this long.
    private val syncCooldownUntil = mutableMapOf<Long, Long>()

    companion object {
@@ -81,7 +89,7 @@ class FileWatchService : Service() {

    override fun onBind(intent: Intent?): IBinder? = null

-    private suspend fun refresh() {
+    private suspend fun refresh() = refreshMutex.withLock {
        clearWatchers()
        val pairs = syncPairDao.getEnabled().filter { it.scheduleType == ScheduleType.ON_CHANGE }

@@ -145,11 +153,41 @@ class FileWatchService : Service() {
            return
        }
        fileObservers[pairId] = mutableListOf()
+        // Set startup cooldown BEFORE registering watchers so inotify events that fire
+        // immediately on registration don't trigger the debounce before catchupScan runs.
+        syncCooldownUntil[pairId] = System.currentTimeMillis() + 15_000
        watchDirRecursive(dir, pairId, wifiOnly, chargingOnly)
        Timber.d("FileWatchService: watching pair $pairId at $path (${fileObservers[pairId]?.size} dirs)")
+        startSyncMonitor(pairId)
        scope.launch { catchupScan(pairId, dir, wifiOnly, chargingOnly) }
    }

+    // Watches WorkManager for ANY sync tagged sync_$pairId (manual, catchup, onchange).
+    // Sets cooldown while running and for 60s after, so FileObserver events from our
+    // own file writes never trigger a re-sync regardless of what started the sync.
+    private fun startSyncMonitor(pairId: Long) {
+        syncMonitorJobs[pairId]?.cancel()
+        syncMonitorJobs[pairId] = scope.launch {
+            var wasSyncing = false
+            WorkManager.getInstance(applicationContext)
+                .getWorkInfosByTagFlow("sync_$pairId")
+                .collect { infos ->
+                    val isSyncing = infos.any {
+                        it.state == WorkInfo.State.RUNNING || it.state == WorkInfo.State.ENQUEUED
+                    }
+                    if (isSyncing) {
+                        Timber.d("FileWatchService: sync active for pair $pairId — cooldown extended")
+                        syncCooldownUntil[pairId] = System.currentTimeMillis() + 120_000
+                        wasSyncing = true
+                    } else if (wasSyncing) {
+                        Timber.d("FileWatchService: sync finished for pair $pairId — 60s settle cooldown")
+                        syncCooldownUntil[pairId] = System.currentTimeMillis() + 60_000
+                        wasSyncing = false
+                    }
+                }
+        }
+    }
+
    private fun watchDirRecursive(dir: File, pairId: Long, wifiOnly: Boolean, chargingOnly: Boolean) {
        if (!dir.isDirectory) return
        val mask = FileObserver.CREATE or FileObserver.DELETE or FileObserver.MODIFY or
@@ -188,27 +226,45 @@ 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<String, Long>()
-        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)
+            // Hold cooldown for duration of sync + 60s settle
+            syncCooldownUntil[pairId] = System.currentTimeMillis() + 120_000
+            val req = SyncWorker.buildOneTimeRequest(pairId, wifiOnly, chargingOnly)
            WorkManager.getInstance(applicationContext)
-                .enqueueUniqueWork(
-                    "catchup_$pairId",
-                    ExistingWorkPolicy.KEEP,
-                    SyncWorker.buildOneTimeRequest(pairId, wifiOnly, chargingOnly),
-                )
+                .enqueueUniqueWork("catchup_$pairId", ExistingWorkPolicy.KEEP, req)
+            scope.launch {
+                try {
+                    WorkManager.getInstance(applicationContext)
+                        .getWorkInfoByIdFlow(req.id)
+                        .first { it?.state?.isFinished == true }
+                    syncCooldownUntil[pairId] = System.currentTimeMillis() + 60_000
+                } catch (e: CancellationException) {
+                    throw e
+                } catch (_: Exception) {
+                    syncCooldownUntil[pairId] = System.currentTimeMillis() + 60_000
+                }
+            }
        }
    }

@@ -223,6 +279,12 @@ class FileWatchService : Service() {
        debounceJobs[pairId]?.cancel()
        debounceJobs[pairId] = scope.launch {
            delay(5_000)
+            // Re-check: catchupScan or another path may have already set a cooldown
+            // and handled this sync while we were waiting.
+            if (System.currentTimeMillis() < (syncCooldownUntil[pairId] ?: 0L)) {
+                Timber.d("FileWatchService: debounce fired but cooldown active for pair $pairId, skipping")
+                return@launch
+            }
            val pair = syncPairDao.getById(pairId)
            if (pair == null || !pair.isEnabled) return@launch
            Timber.d("FileWatchService: triggering sync for pair $pairId after debounce")
@@ -253,8 +315,10 @@ class FileWatchService : Service() {
                    }
                    delay(12_000)
                    updateNotificationDynamic(null)
+                } catch (e: CancellationException) {
+                    throw e
                } catch (_: Exception) {
-                    syncCooldownUntil[pairId] = 0L
+                    syncCooldownUntil[pairId] = System.currentTimeMillis() + 60_000
                    updateNotificationDynamic(null)
                }
            }
@@ -268,6 +332,8 @@ class FileWatchService : Service() {
        contentObservers.clear()
        debounceJobs.values.forEach { it.cancel() }
        debounceJobs.clear()
+        syncMonitorJobs.values.forEach { it.cancel() }
+        syncMonitorJobs.clear()
        syncCooldownUntil.clear()
    }

@@ -1,25 +1,12 @@
 <?xml version="1.0" encoding="utf-8"?>
 <vector xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:aapt="http://schemas.android.com/aapt"
    android:width="108dp"
    android:height="108dp"
    android:viewportWidth="108"
    android:viewportHeight="108">

-    <!-- Dark charcoal background, matching Avast-style dark icon bg -->
+    <!-- Pure black background -->
    <path android:pathData="M0,0 H108 V108 H0 Z"
-        android:fillColor="#1F1F2E"/>
-
-    <!-- Very subtle inner glow -->
-    <path android:pathData="M0,0 H108 V108 H0 Z"
-        android:fillAlpha="0.25">
-        <aapt:attr name="android:fillColor">
-            <gradient android:type="radial"
-                android:gradientRadius="60"
-                android:centerX="54" android:centerY="50"
-                android:startColor="#3D3A50"
-                android:endColor="#00000000"/>
-        </aapt:attr>
-    </path>
+        android:fillColor="#000000"/>

 </vector>
@@ -1,67 +1,84 @@
 <?xml version="1.0" encoding="utf-8"?>
-<!--
-  SyncFlow icon foreground.
-  Three bold teardrop shapes in Avast-palette colors (teal, red, amber),
-  tips meeting at center (54,54), wide heads pointing outward at 0/120/240 deg.
-  White cloud centred over the intersection point.
-->
 <vector xmlns:android="http://schemas.android.com/apk/res/android"
    android:width="108dp"
    android:height="108dp"
    android:viewportWidth="108"
    android:viewportHeight="108">

-    <!-- Teal teardrop — head pointing straight up -->
-    <group android:rotation="0"
-        android:pivotX="54"
-        android:pivotY="54">
-        <path
-            android:fillColor="#00C4A7"
-            android:pathData="M 54,57 C 50,57 38,52 34,41 C 30,30 38,20 54,20 C 70,20 78,30 74,41 C 70,52 58,57 54,57 Z"/>
-    </group>
+    <!--
+      Four thick arcs arranged as an interlocked pinwheel.
+      Each arc sweeps ~210 degrees, rounded caps, radius 18 from center (54,54).
+      Draw order creates natural over/under at the four crossing points:
+        blue under green, green under red, red under orange, orange under blue (re-draw blue tip).

-    <!-- Red teardrop — head pointing lower-right (120 deg CW from up) -->
-    <group android:rotation="120"
-        android:pivotX="54"
-        android:pivotY="54">
-        <path
-            android:fillColor="#F44336"
-            android:pathData="M 54,57 C 50,57 38,52 34,41 C 30,30 38,20 54,20 C 70,20 78,30 74,41 C 70,52 58,57 54,57 Z"/>
-    </group>
+      Arc endpoints computed at radius 18, sweep 210 deg clockwise:
+        start angle  end angle    start point    end point
+        270 (top)    120          (54, 36)        (45, 70)
+          0 (right)  210          (72, 54)        (39, 45)
+         90 (bot)    300          (54, 72)        (63, 38)
+        180 (left)   390=30       (36, 54)        (69, 63)
+    -->

-    <!-- Amber teardrop — head pointing lower-left (240 deg CW from up) -->
-    <group android:rotation="240"
-        android:pivotX="54"
-        android:pivotY="54">
-        <path
-            android:fillColor="#FFC107"
-            android:pathData="M 54,57 C 50,57 38,52 34,41 C 30,30 38,20 54,20 C 70,20 78,30 74,41 C 70,52 58,57 54,57 Z"/>
-    </group>
+    <!-- Blue — starts at top, sweeps clockwise to lower-left -->
+    <path
+        android:strokeColor="#2979FF"
+        android:strokeWidth="8.5"
+        android:fillColor="#00000000"
+        android:strokeLineCap="round"
+        android:pathData="M 54,36 A 18,18 0 1,1 45,70"/>

-    <!-- White cloud centred at (54,52), sits over the teardrop intersection -->
+    <!-- Green — starts at bottom, sweeps clockwise to upper-right -->
+    <path
+        android:strokeColor="#00C853"
+        android:strokeWidth="8.5"
+        android:fillColor="#00000000"
+        android:strokeLineCap="round"
+        android:pathData="M 54,72 A 18,18 0 1,1 63,38"/>
+
+    <!-- Red — starts at right, sweeps clockwise to lower-left -->
+    <path
+        android:strokeColor="#E53935"
+        android:strokeWidth="8.5"
+        android:fillColor="#00000000"
+        android:strokeLineCap="round"
+        android:pathData="M 72,54 A 18,18 0 1,1 39,45"/>
+
+    <!-- Orange — starts at left, sweeps clockwise to upper-right -->
+    <path
+        android:strokeColor="#FF6D00"
+        android:strokeWidth="8.5"
+        android:fillColor="#00000000"
+        android:strokeLineCap="round"
+        android:pathData="M 36,54 A 18,18 0 1,1 69,63"/>
+
+    <!-- Re-draw blue start cap on top so it goes OVER orange end -->
+    <path
+        android:strokeColor="#2979FF"
+        android:strokeWidth="8.5"
+        android:fillColor="#00000000"
+        android:strokeLineCap="round"
+        android:pathData="M 54,36 A 18,18 0 0,1 62,37.5"/>
+
+    <!-- White sync circle at center -->
+    <path
+        android:fillColor="#000000"
+        android:pathData="M 45,54 A 9,9 0 1,0 63,54 A 9,9 0 1,0 45,54 Z"/>
+
+    <!-- Sync ring -->
+    <path
+        android:strokeColor="#FFFFFF"
+        android:strokeWidth="2.5"
+        android:fillColor="#00000000"
+        android:pathData="M 46.5,54 A 7.5,7.5 0 1,0 61.5,54 A 7.5,7.5 0 1,0 46.5,54 Z"/>
+
+    <!-- Top arrow head (pointing up) -->
    <path
        android:fillColor="#FFFFFF"
-        android:pathData="
-          M 42,62
-          A 8,8 0 0,1 42,46
-          A 8,8 0 0,1 51,39
-          A 11,11 0 0,1 67,42
-          A 7,7 0 0,1 68,56
-          A 7,7 0 0,1 66,62
-          Z"/>
+        android:pathData="M 54,46.5 L 57,50.5 L 51,50.5 Z"/>

-    <!-- Cloud inner shadow to make it pop from the coloured teardrops -->
+    <!-- Bottom arrow head (pointing down) -->
    <path
-        android:fillColor="#00000000"
-        android:strokeColor="#18000000"
-        android:strokeWidth="1.5"
-        android:pathData="
-          M 42,62
-          A 8,8 0 0,1 42,46
-          A 8,8 0 0,1 51,39
-          A 11,11 0 0,1 67,42
-          A 7,7 0 0,1 68,56
-          A 7,7 0 0,1 66,62
-          Z"/>
+        android:fillColor="#FFFFFF"
+        android:pathData="M 54,61.5 L 51,57.5 L 57,57.5 Z"/>

 </vector>
@@ -1,2 +1,2 @@
-VERSION_NAME=1.0.29
-VERSION_CODE=30
+VERSION_NAME=1.0.37
+VERSION_CODE=38
Author	SHA1	Message	Date
amir	897b685c70	Fix perpetual sync loop and wrong delete decisions 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 <noreply@anthropic.com>	2026-05-25 15:13:43 +00:00
amir	4b20697bb1	v1.0.32: fix manual sync loop via WorkManager tag monitor Root cause: manual sync triggered from the UI had no cooldown set in FileWatchService, so file writes during any manual sync fired FileObserver → debounce → another sync → loop. Fix: startSyncMonitor() subscribes to getWorkInfosByTagFlow("sync_$pairId") and watches ALL sync work for each pair — manual, catchup, onchange — via the tag that SyncWorker.buildOneTimeRequest() always adds. - When any sync is RUNNING or ENQUEUED: cooldown extended to now+120s - When sync transitions from running to finished: 60s settle cooldown - Monitor job stored in syncMonitorJobs map and cancelled in clearWatchers() This means no matter what triggers a sync, FileObserver events from the resulting file writes are always suppressed until the folder settles. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-25 14:11:58 +00:00
amir	66d28761a8	v1.0.31: fix remaining sync loop triggers + icon redesign Three additional fixes found via live device logs: 1. Startup race window: FileObserver fires immediately after startWatching() before catchupScan coroutine runs, starting a 5s debounce with cooldown=0. Fixed by setting a 15s startup cooldown in watchPath() BEFORE calling watchDirRecursive. 2. Stale debounce bypass: debounce job started with cooldown=0 fires 5s later even after catchupScan has already set cooldown and started a catchup sync. Fixed by re-checking cooldown after the 5s delay and aborting if already active. 3. Debounce not cancelled by catchupScan: if a debounce was queued before catchupScan ran, catchupScan would enqueue a catchup sync AND the old debounce would fire 5s later enqueuing a second sync. Fixed by cancelling pending debounce in catchupScan before enqueue. Icon: four thick arcs (blue/red/green/orange) in a 4-way pinwheel with over/under ordering. White sync-arrow circle at center. Pure black background. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-25 14:06:19 +00:00
amir	ec478531da	v1.0.30: fix sync loop root causes + icon redesign Three root causes found via live logcat on device: 1. concurrent refresh() race: onStartCommand received twice causes two refresh() coroutines to run in parallel, doubling FileObserver and catchupScan registrations. Fixed with Mutex.withLock on refresh(). 2. catchupScan no cooldown: catchup syncs write files but never set syncCooldownUntil, so every written file immediately re-triggers onChangeDetected. Fixed by setting cooldown before enqueue and watching work completion same as onChangeDetected does. 3. CancellationException caught silently: exception handler catch(_: Exception) was catching CancellationException and resetting cooldown to 0L, re-opening the loop. Fixed by rethrowing CancellationException and setting 60s cooldown on other errors. Icon: interlocked rings (blue/red/green/orange) with sync arrow at center, pure black background — matches reference image. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-25 13:48:18 +00:00