fix(sync): isolate unreadable local items instead of stopping folder sync

changes/unreleased/489-sync-unreadable-local-items.md

@@ -0,0 +1,7 @@
+category: fix
+issue: 489
+pull: none
+platforms: desktop
+user-facing: yes
+
+Desktop folder sync, including Git working trees, now skips and lists a local item that vanishes, is locked, or is not a regular file instead of stopping the whole folder.

tools/kotlin-file-size-baseline.txt

@@ -46,7 +46,7 @@ ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/nativeui/runtime/NativeColl
 ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/nativeui/runtime/NativeRecordActionsTest.kt|3161
 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopAppUpdates.kt|925
 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileReadCache.kt|1536
-ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncEngine.kt|884
+ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncEngine.kt|876
 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopFileSyncRemoteTree.kt|857
 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopNextcloudServices.kt|6169
 ui/src/desktopMain/kotlin/dev/obiente/nextcloudnative/app/DesktopVirtualRangeCache.kt|2759

ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileSyncCoordinator.kt

@@ -878,10 +878,10 @@ private fun requireValidFileSyncPair(pair: FileSyncPair) {
                 "The persisted sync decision choices do not match the pair direction."
             }
         }
-        val expectedOperation = if (resolved != null) {
-            resolveDecisionOperation(pair, work, resolved.choice)
-        } else {
-            planFileSync(
+        val expectedOperation = when {
+            resolved != null -> resolveDecisionOperation(pair, work, resolved.choice)
+            work.isUnavailableLocalItemReport() -> work.operation
+            else -> planFileSync(
                 localEntries = listOfNotNull(work.observedLocal),
                 remoteEntries = listOfNotNull(work.observedRemote),
                 baselines = listOfNotNull(work.observedBaseline),

ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/FileSyncLocalAvailability.kt

@@ -0,0 +1,159 @@
+package dev.obiente.nextcloudnative.app
+
+/** Why a platform scan could not observe one local item. */
+enum class FileSyncLocalUnavailableReason {
+    /** The item disappeared or was replaced between directory listing and reading. */
+    Vanished,
+
+    /** The item exists but could not be read, for example while another app holds it open. */
+    Unreadable,
+
+    /** The item is not a regular file or folder, for example a socket, device, or reparse point. */
+    Unsupported,
+}
+
+/**
+ * A local path whose current state is unknown for one scan.
+ *
+ * The item and every descendant are withheld from planning, so an unreadable subtree is never
+ * interpreted as a local deletion, a missing file to restore, or new content to upload.
+ */
+data class FileSyncUnavailableLocalItem(
+    val relativePath: String,
+    val reason: FileSyncLocalUnavailableReason,
+) {
+    init {
+        requireValidSyncPath(relativePath)
+        require(relativePath.length <= MAX_FILE_SYNC_PATH_LENGTH)
+    }
+}
+
+/** Local and remote observations that remain after unavailable local subtrees are withheld. */
+data class FileSyncAvailableObservations(
+    val localEntries: List<LocalSyncEntry>,
+    val remoteEntries: List<RemoteSyncEntry>,
+    val unavailable: List<FileSyncUnavailableLocalItem>,
+)
+
+/**
+ * Withholds every local and remote observation at or below an unavailable local path.
+ *
+ * Persisted baselines are untouched, so a later complete scan resumes normal planning. Nested
+ * reports collapse into their outermost unavailable ancestor.
+ */
+fun withholdUnavailableFileSyncPaths(
+    localEntries: List<LocalSyncEntry>,
+    remoteEntries: List<RemoteSyncEntry>,
+    unavailable: List<FileSyncUnavailableLocalItem>,
+): FileSyncAvailableObservations {
+    val withheldPaths = HashSet<String>()
+    fun withheld(path: String): Boolean = path.syncPathAndAncestors().any { it in withheldPaths }
+    // Shallower paths first, so every nested report finds its already-withheld ancestor.
+    val outermost = unavailable
+        .sortedWith(compareBy({ it.relativePath.count { char -> char == '/' } }, { it.relativePath }))
+        .filter { item -> !withheld(item.relativePath) && withheldPaths.add(item.relativePath) }
+        .sortedBy(FileSyncUnavailableLocalItem::relativePath)
+    if (outermost.isEmpty()) return FileSyncAvailableObservations(localEntries, remoteEntries, emptyList())
+    fun available(path: String) = !withheld(path)
+    return FileSyncAvailableObservations(
+        localEntries = localEntries.filter { available(it.relativePath) },
+        remoteEntries = remoteEntries.filter { available(it.relativePath) },
+        unavailable = outermost,
+    )
+}
+
+/**
+ * Narrows a scan predicate so remote traversal never descends into unavailable local subtrees.
+ *
+ * Their server content cannot take part in planning, so listing it would only add network work
+ * and listing failures. [withholdUnavailableFileSyncPaths] still applies as the second layer.
+ */
+fun ((relativePath: String, kind: SyncEntryKind) -> Boolean).excludingUnavailableFileSyncPaths(
+    unavailable: List<FileSyncUnavailableLocalItem>,
+): (relativePath: String, kind: SyncEntryKind) -> Boolean {
+    if (unavailable.isEmpty()) return this
+    val withheldPaths = unavailable.mapTo(HashSet(), FileSyncUnavailableLocalItem::relativePath)
+    return { path, kind -> path.syncPathAndAncestors().none { it in withheldPaths } && this(path, kind) }
+}
+
+/**
+ * Records each unavailable local item as visible skipped work after planning.
+ *
+ * At most [maximumReports] items become work items; the run summary reports the complete count.
+ */
+fun FileSyncCoordinatorState.withUnavailableFileSyncReports(
+    pairId: String,
+    unavailable: List<FileSyncUnavailableLocalItem>,
+    maximumReports: Int = MAX_FILE_SYNC_UNAVAILABLE_REPORTS,
+): FileSyncCoordinatorState {
+    require(maximumReports in 0..MAX_FILE_SYNC_UNAVAILABLE_REPORTS)
+    if (unavailable.isEmpty() || maximumReports == 0) return this
+    return copy(
+        pairs = pairs.map { pair ->
+            if (pair.id != pairId) return@map pair
+            val plannedPaths = pair.workItems.mapTo(HashSet(), FileSyncWorkItem::relativePath)
+            var nextId = pair.nextWorkId
+            val reports = unavailable.asSequence()
+                .filter { it.relativePath !in plannedPaths }
+                .take(maximumReports)
+                .map { item ->
+                    require(nextId < Long.MAX_VALUE) { "The sync work ID space is exhausted." }
+                    FileSyncWorkItem(
+                        id = nextId++,
+                        relativePath = item.relativePath,
+                        observedLocal = null,
+                        observedRemote = null,
+                        observedBaseline = null,
+                        operation = FileSyncOperation.Skipped(item.relativePath, item.skippedReason()),
+                        state = FileSyncExecutionState.Skipped,
+                    )
+                }
+                .toList()
+            pair.copy(workItems = pair.workItems + reports, nextWorkId = nextId)
+        },
+    )
+}
+
+/**
+ * True for a skipped report produced by [withUnavailableFileSyncReports]. Such work has no observed
+ * generation to re-plan, so persisted validation instead requires an unexecuted item whose reason
+ * is exactly one generated for its path.
+ */
+internal fun FileSyncWorkItem.isUnavailableLocalItemReport(): Boolean {
+    val skipped = operation as? FileSyncOperation.Skipped ?: return false
+    return state == FileSyncExecutionState.Skipped &&
+        observedLocal == null && observedRemote == null && observedBaseline == null &&
+        decision == null && attemptCount == 0 && lastAttemptEpochMillis == null &&
+        uploadCheckpoint == null && !contentMismatchVerified &&
+        FileSyncLocalUnavailableReason.entries.any { reason ->
+            FileSyncUnavailableLocalItem(relativePath, reason).skippedReason() == skipped.reason
+        }
+}
+
+/** One reviewable sentence per item; the path keeps each unavailable item individually visible. */
+internal fun FileSyncUnavailableLocalItem.skippedReason(): String {
+    val path = relativePath.boundedForSyncReason()
+    return when (reason) {
+        FileSyncLocalUnavailableReason.Vanished ->
+            "Not synced this time: $path disappeared or changed while the folder was checked. " +
+                "It will be checked again on the next sync."
+        FileSyncLocalUnavailableReason.Unreadable ->
+            "Not synced this time: $path could not be read. It may be open in another app or " +
+                "access may be denied. It will be checked again on the next sync."
+        FileSyncLocalUnavailableReason.Unsupported ->
+            "Not synced: $path is not a regular file or folder."
+    }
+}
+
+private fun String.boundedForSyncReason(): String {
+    if (length <= MAX_FILE_SYNC_REASON_PATH_LENGTH) return this
+    val keep = (MAX_FILE_SYNC_REASON_PATH_LENGTH - 3) / 2
+    return take(keep) + "..." + takeLast(keep)
+}
+
+private fun String.syncPathAndAncestors(): Sequence<String> =
+    generateSequence(this) { path -> path.substringBeforeLast('/', "").takeIf(String::isNotEmpty) }
+
+/** Visible skipped-work reports kept per run; every unavailable path is withheld regardless. */
+internal const val MAX_FILE_SYNC_UNAVAILABLE_REPORTS = 1_000
+private const val MAX_FILE_SYNC_REASON_PATH_LENGTH = 600

ui/src/commonTest/kotlin/dev/obiente/nextcloudnative/app/FileSyncLocalAvailabilityTest.kt

@@ -0,0 +1,191 @@
+package dev.obiente.nextcloudnative.app
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertIs
+import kotlin.test.assertTrue
+
+class FileSyncLocalAvailabilityTest {
+    @Test
+    fun `unavailable local subtrees are withheld from both sides and nested reports collapse`() {
+        val observed = withholdUnavailableFileSyncPaths(
+            localEntries = listOf(
+                local(".git", SyncEntryKind.Directory),
+                local(".git/objects", SyncEntryKind.Directory),
+                local(".git/objects/ab", SyncEntryKind.Directory),
+                local(".git/HEAD"),
+                local(".git/objects pack"),
+                local("README.md"),
+            ),
+            remoteEntries = listOf(
+                remote(".git/objects", SyncEntryKind.Directory),
+                remote(".git/objects/ab/cdef"),
+                remote(".git/index.lock"),
+                remote(".git/HEAD"),
+            ),
+            unavailable = listOf(
+                FileSyncUnavailableLocalItem(".git/objects/ab", FileSyncLocalUnavailableReason.Vanished),
+                FileSyncUnavailableLocalItem(".git/objects", FileSyncLocalUnavailableReason.Unreadable),
+                FileSyncUnavailableLocalItem(".git/index.lock", FileSyncLocalUnavailableReason.Vanished),
+            ),
+        )
+
+        assertEquals(
+            listOf(".git", ".git/HEAD", ".git/objects pack", "README.md"),
+            observed.localEntries.map(LocalSyncEntry::relativePath),
+        )
+        assertEquals(listOf(".git/HEAD"), observed.remoteEntries.map(RemoteSyncEntry::relativePath))
+        assertEquals(
+            listOf(
+                FileSyncUnavailableLocalItem(".git/index.lock", FileSyncLocalUnavailableReason.Vanished),
+                FileSyncUnavailableLocalItem(".git/objects", FileSyncLocalUnavailableReason.Unreadable),
+            ),
+            observed.unavailable,
+        )
+    }
+
+    @Test
+    fun `an unreadable synced git file is never planned as a deletion and is reported`() {
+        val configuration = FileSyncConfiguration(
+            deviceLabel = "Workstation",
+            deletionPolicy = FileSyncDeletionPolicy.Propagate,
+        )
+        val pair = FileSyncPair(
+            id = "pair",
+            accountId = "account",
+            localRootId = "root",
+            remoteRootPath = "Repo",
+            configuration = configuration,
+            baselines = listOf(
+                FileSyncBaseline(".git", SyncEntryKind.Directory, "local-dir", "remote-dir"),
+                FileSyncBaseline(".git/index", SyncEntryKind.File, "local-index", "remote-index"),
+                FileSyncBaseline(".git/objects", SyncEntryKind.Directory, "local-objects", "remote-objects"),
+                FileSyncBaseline(".git/objects/ab", SyncEntryKind.File, "local-object", "remote-object"),
+            ),
+        )
+        val unavailable = listOf(
+            FileSyncUnavailableLocalItem(".git/index", FileSyncLocalUnavailableReason.Unreadable),
+            FileSyncUnavailableLocalItem(".git/objects", FileSyncLocalUnavailableReason.Unreadable),
+        )
+        val observed = withholdUnavailableFileSyncPaths(
+            localEntries = listOf(
+                LocalSyncEntry(".git", SyncEntryKind.Directory, "local-dir"),
+                local(".git/COMMIT_EDITMSG"),
+            ),
+            remoteEntries = listOf(
+                RemoteSyncEntry(".git", SyncEntryKind.Directory, "remote-dir"),
+                RemoteSyncEntry(".git/index", SyncEntryKind.File, "remote-index"),
+                RemoteSyncEntry(".git/objects", SyncEntryKind.Directory, "remote-objects"),
+                RemoteSyncEntry(".git/objects/ab", SyncEntryKind.File, "remote-object"),
+            ),
+            unavailable = unavailable,
+        )
+
+        val scanned = scanFileSyncPair(
+            FileSyncCoordinatorState(listOf(pair)),
+            pair.id,
+            observed.localEntries,
+            observed.remoteEntries,
+            nowEpochMillis = 10L,
+            maximumWorkItems = MAX_FILE_SYNC_WORK_ITEMS - observed.unavailable.size,
+        ).withUnavailableFileSyncReports(pair.id, observed.unavailable).pairs.single()
+
+        val operations = scanned.workItems.associate { it.relativePath to it.operation }
+        assertIs<FileSyncOperation.Upload>(operations.getValue(".git/COMMIT_EDITMSG"))
+        assertTrue(operations.values.none { it is FileSyncOperation.DeleteLocal || it is FileSyncOperation.DeleteRemote })
+        val indexReport = assertIs<FileSyncOperation.Skipped>(operations.getValue(".git/index"))
+        assertTrue(indexReport.reason.contains(".git/index could not be read"))
+        assertIs<FileSyncOperation.Skipped>(operations.getValue(".git/objects"))
+        assertEquals(pair.baselines, scanned.baselines)
+        assertEquals(
+            setOf(".git/index", ".git/objects"),
+            scanned.workItems.filter { it.state == FileSyncExecutionState.Skipped }.map { it.relativePath }.toSet(),
+        )
+        assertTrue(scanned.workItems.all { it.id < scanned.nextWorkId })
+
+        val recovered = scanFileSyncPair(
+            FileSyncCoordinatorState(listOf(scanned)),
+            pair.id,
+            localEntries = listOf(
+                LocalSyncEntry(".git", SyncEntryKind.Directory, "local-dir"),
+                local(".git/COMMIT_EDITMSG"),
+                LocalSyncEntry(".git/index", SyncEntryKind.File, "local-index"),
+                LocalSyncEntry(".git/objects", SyncEntryKind.Directory, "local-objects"),
+                LocalSyncEntry(".git/objects/ab", SyncEntryKind.File, "local-object"),
+            ),
+            remoteEntries = observed.remoteEntries + listOf(
+                RemoteSyncEntry(".git/index", SyncEntryKind.File, "remote-index"),
+                RemoteSyncEntry(".git/objects", SyncEntryKind.Directory, "remote-objects"),
+                RemoteSyncEntry(".git/objects/ab", SyncEntryKind.File, "remote-object"),
+            ),
+            nowEpochMillis = 20L,
+        ).pairs.single()
+
+        assertEquals(listOf(".git/COMMIT_EDITMSG"), recovered.workItems.map(FileSyncWorkItem::relativePath))
+    }
+
+    @Test
+    fun `reports are bounded skip planned paths and keep reasons within persisted limits`() {
+        val pair = FileSyncPair(
+            id = "pair",
+            accountId = "account",
+            localRootId = "root",
+            remoteRootPath = "Repo",
+            configuration = FileSyncConfiguration(deviceLabel = "Workstation"),
+        )
+        val planned = scanFileSyncPair(
+            FileSyncCoordinatorState(listOf(pair)),
+            pair.id,
+            localEntries = listOf(local("README.md")),
+            remoteEntries = emptyList(),
+            nowEpochMillis = 10L,
+        )
+        val longName = "segment-".repeat(200).trimEnd('-')
+        val unavailable = listOf(
+            FileSyncUnavailableLocalItem("README.md", FileSyncLocalUnavailableReason.Vanished),
+            FileSyncUnavailableLocalItem(".git/$longName", FileSyncLocalUnavailableReason.Unsupported),
+            FileSyncUnavailableLocalItem(".git/index.lock", FileSyncLocalUnavailableReason.Vanished),
+        )
+
+        val reported = planned.withUnavailableFileSyncReports(pair.id, unavailable, maximumReports = 1)
+            .pairs.single()
+
+        assertEquals(listOf("README.md", ".git/$longName"), reported.workItems.map(FileSyncWorkItem::relativePath))
+        val reason = (reported.workItems.last().operation as FileSyncOperation.Skipped).reason
+        assertTrue(reason.length <= MAX_FILE_SYNC_FAILURE_LENGTH)
+        assertTrue(reason.contains("..."))
+        assertTrue(reason.endsWith("is not a regular file or folder."))
+        assertEquals(planned, planned.withUnavailableFileSyncReports(pair.id, emptyList()))
+    }
+
+    @Test
+    fun `persisted pairs accept generated reports and reject unobserved skips with other reasons`() {
+        val pair = FileSyncPair(
+            id = "pair",
+            accountId = "account",
+            localRootId = "root",
+            remoteRootPath = "Repo",
+            configuration = FileSyncConfiguration(deviceLabel = "Workstation"),
+        )
+        val item = FileSyncUnavailableLocalItem(".git/index.lock", FileSyncLocalUnavailableReason.Vanished)
+        val reported = FileSyncCoordinatorState(listOf(pair)).withUnavailableFileSyncReports(pair.id, listOf(item))
+            .pairs.single()
+        val report = reported.workItems.single()
+
+        assertEquals(reported, reported.copy())
+        assertFailsWith<IllegalArgumentException> {
+            reported.copy(
+                workItems = listOf(
+                    report.copy(operation = FileSyncOperation.Skipped(report.relativePath, "Forged skip.")),
+                ),
+            )
+        }
+    }
+
+    private fun local(path: String, kind: SyncEntryKind = SyncEntryKind.File) =
+        LocalSyncEntry(path, kind, "local-$path", size = if (kind == SyncEntryKind.File) 1L else null)
+
+    private fun remote(path: String, kind: SyncEntryKind = SyncEntryKind.File) =
+        RemoteSyncEntry(path, kind, "remote-$path", size = if (kind == SyncEntryKind.File) 1L else null)
+}