Compare commits

...

2 Commits

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
4 changed files with 57 additions and 24 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,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
@@ -44,8 +46,10 @@ class FileWatchService : Service() {
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 {
@@ -154,9 +158,36 @@ class FileWatchService : Service() {
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
@@ -195,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<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)
@@ -297,6 +332,8 @@ class FileWatchService : Service() {
contentObservers.clear()
debounceJobs.values.forEach { it.cancel() }
debounceJobs.clear()
syncMonitorJobs.values.forEach { it.cancel() }
syncMonitorJobs.clear()
syncCooldownUntil.clear()
}
+2 -2
View File
@@ -1,2 +1,2 @@
VERSION_NAME=1.0.31
VERSION_CODE=32
VERSION_NAME=1.0.37
VERSION_CODE=38