diff --git a/aw-server-rust b/aw-server-rust index 97f51e30..3d3b9dd1 160000 --- a/aw-server-rust +++ b/aw-server-rust @@ -1 +1 @@ -Subproject commit 97f51e3078e685c9d188bc80d7b37d55cb00848f +Subproject commit 3d3b9dd106f185e5d1f7d90bf336e548261f9047 diff --git a/mobile/src/main/java/net/activitywatch/android/AWPreferences.kt b/mobile/src/main/java/net/activitywatch/android/AWPreferences.kt index 140cf720..f68d8f4b 100644 --- a/mobile/src/main/java/net/activitywatch/android/AWPreferences.kt +++ b/mobile/src/main/java/net/activitywatch/android/AWPreferences.kt @@ -93,6 +93,21 @@ class AWPreferences(context: Context) { success = sharedPreferences.getBoolean("lastSyncSucceeded", false), error = sharedPreferences.getString("lastSyncError", null) ?.takeIf { it.isNotEmpty() }, + summary = sharedPreferences.getString("lastSyncSummary", null) + ?.takeIf { it.isNotEmpty() }, + hasReport = sharedPreferences.getBoolean("lastSyncHasReport", false), + eventsPulled = sharedPreferences.getInt("lastSyncEventsPulled", 0), + eventsPushed = sharedPreferences.getInt("lastSyncEventsPushed", 0), + peersImported = sharedPreferences.getInt("lastSyncPeersImported", 0), + peersSkipped = sharedPreferences.getInt("lastSyncPeersSkipped", 0), + peersFailed = sharedPreferences.getInt("lastSyncPeersFailed", 0), + // Warnings are normalized to single lines before they get here, so a + // newline join round-trips exactly and unlike a JSON array it cannot + // fail to parse. + warnings = sharedPreferences.getString("lastSyncWarnings", null) + ?.split("\n") + ?.filter { it.isNotEmpty() } + ?: emptyList(), ) } @@ -100,12 +115,29 @@ class AWPreferences(context: Context) { val editor = sharedPreferences.edit() .putLong("lastSyncCompletedAt", status.completedAt) .putBoolean("lastSyncSucceeded", status.success) + .putBoolean("lastSyncHasReport", status.hasReport) + .putInt("lastSyncEventsPulled", status.eventsPulled) + .putInt("lastSyncEventsPushed", status.eventsPushed) + .putInt("lastSyncPeersImported", status.peersImported) + .putInt("lastSyncPeersSkipped", status.peersSkipped) + .putInt("lastSyncPeersFailed", status.peersFailed) val error = status.error?.takeIf { it.isNotEmpty() } if (error == null) { editor.remove("lastSyncError") } else { editor.putString("lastSyncError", error) } + val summary = status.summary?.takeIf { it.isNotEmpty() } + if (summary == null) { + editor.remove("lastSyncSummary") + } else { + editor.putString("lastSyncSummary", summary) + } + if (status.warnings.isEmpty()) { + editor.remove("lastSyncWarnings") + } else { + editor.putString("lastSyncWarnings", status.warnings.joinToString("\n")) + } editor.apply() appContext.sendBroadcast( android.content.Intent(LAST_SYNC_STATUS_CHANGED_ACTION).setPackage(appContext.packageName) diff --git a/mobile/src/main/java/net/activitywatch/android/SyncInterface.kt b/mobile/src/main/java/net/activitywatch/android/SyncInterface.kt index 9a28cc07..7bd048ed 100644 --- a/mobile/src/main/java/net/activitywatch/android/SyncInterface.kt +++ b/mobile/src/main/java/net/activitywatch/android/SyncInterface.kt @@ -23,9 +23,36 @@ data class SyncStatus( // JNI already returns {"success": false, "error": "..."}; keep a bounded // copy so the settings line can say why, not just that it failed. val error: String? = null, + // Per-run facts from aw-sync's SyncReport. A boolean cannot tell "moved a + // million events" from "did nothing"; these can. Every field defaults to + // the pre-SyncReport payload (activitywatch/aw-server-rust#699), so an + // older bundled native lib still parses. + val summary: String? = null, + // True only when the payload actually carried a SyncReport. The counts default + // to 0, which is indistinguishable from a real no-op pass, so the renderer + // keys off this instead of the values. + val hasReport: Boolean = false, + val eventsPulled: Int = 0, + val eventsPushed: Int = 0, + val peersImported: Int = 0, + val peersSkipped: Int = 0, + val peersFailed: Int = 0, + val warnings: List = emptyList(), ) { companion object { const val MAX_ERROR_CHARS = 500 + const val MAX_WARNINGS = 5 + // Count keys only. `warnings` is deliberately excluded: it is not a + // count, so a payload carrying warnings but no counts would mark + // hasReport and render an invented "pulled 0, pushed 0" line for a + // pass that never reported its numbers. + private val REPORT_KEYS = listOf( + "events_pulled", + "events_pushed", + "peers_imported", + "peers_skipped", + "peers_failed", + ) private val WHITESPACE = Regex("\\s+") fun normalizeError(raw: String?): String? = @@ -33,6 +60,53 @@ data class SyncStatus( ?.replace(WHITESPACE, " ") ?.take(MAX_ERROR_CHARS) ?.ifBlank { null } + + /** + * Parse a JNI sync response into a status. + * + * Never throws: an unreadable response becomes a failure carrying the + * raw text, rather than propagating an exception out of the sync path. + */ + fun fromJniResponse(response: String, completedAt: Long): SyncStatus { + val json = try { + JSONObject(response) + } catch (e: Exception) { + return SyncStatus( + completedAt = completedAt, + success = false, + error = normalizeError("Unreadable sync response: ${e.message}"), + ) + } + + val success = json.optBoolean("success", false) + val warnings = json.optJSONArray("warnings")?.let { arr -> + (0 until arr.length()) + .mapNotNull { i -> normalizeError(arr.optString(i, "")) } + // Cap after blank-filtering: capping first would drop a + // meaningful warning trailing five blank entries. + .take(MAX_WARNINGS) + } ?: emptyList() + + return SyncStatus( + completedAt = completedAt, + success = success, + error = if (success) { + null + } else { + normalizeError(json.optString("error", "")) ?: "sync failed" + }, + summary = normalizeError(json.optString("message", "")), + // Any report field counts: a push-only pass carries events_pushed + // and peer counts but may omit events_pulled. + hasReport = REPORT_KEYS.any { json.has(it) }, + eventsPulled = json.optInt("events_pulled", 0).coerceAtLeast(0), + eventsPushed = json.optInt("events_pushed", 0).coerceAtLeast(0), + peersImported = json.optInt("peers_imported", 0).coerceAtLeast(0), + peersSkipped = json.optInt("peers_skipped", 0).coerceAtLeast(0), + peersFailed = json.optInt("peers_failed", 0).coerceAtLeast(0), + warnings = warnings, + ) + } } } @@ -144,13 +218,8 @@ class SyncInterface(context: Context) { "Full Sync", { success, message -> syncInFlight.set(false) - AWPreferences(appContext).setLastSyncStatus( - SyncStatus( - completedAt = System.currentTimeMillis(), - success = success, - error = if (success) null else SyncStatus.normalizeError(message), - ) - ) + // Status persistence now happens in performSyncAsync, which has the + // parsed SyncReport; building it here again would discard the counts. callback(success, message) }, mirrorBeforeCallback @@ -192,29 +261,59 @@ class SyncInterface(context: Context) { executor.execute { Log.i(TAG, "Starting sync operation: $operation") + // Native-sync report kept for the catch path: when mirroring fails after + // a successful sync, the failure status must still carry the report + // (counts/warnings) instead of erasing what the pass actually did. + var nativeStatus: SyncStatus? = null try { val response = syncFn() - val json = JSONObject(response) - val success = json.getBoolean("success") + val status = SyncStatus.fromJniResponse(response, System.currentTimeMillis()) + nativeStatus = status + val success = status.success val message = if (success) { - json.getString("message") + status.summary ?: "sync completed" } else { - json.getString("error") + status.error ?: "sync failed" } - // Keep completion feedback honest: a configured SAF directory is part of a - // successful Android sync, so mirror failures must reach the user instead of - // being logged as a non-fatal success. Full-sync callers wait for mirroring. + // Single choke point for status persistence: every sync operation runs + // through here, and this is the only place that holds the SyncReport + // returned across the JNI boundary. Persisting per-caller (as the full-sync + // path used to) is what left pull/push runs unrecorded. + // + // A configured SAF directory is part of a successful Android sync, so for + // full syncs the persist happens only AFTER mirroring completes: the + // settings UI must never show a completed sync while the mirror is still + // running. Non-mirroring operations persist immediately as before. Log.i(TAG, "$operation completed: success=$success, message=$message") if (success && mirrorBeforeCallback) { mirrorSyncFilesToSafDir() } + persistSyncStatus(status) handler.post { callback(success, message) } } catch (e: Exception) { - val errorMsg = "Exception: ${e.message}" + val native = nativeStatus + val status = if (native != null && native.success) { + // The native sync itself completed; this failure came from the + // post-sync step (mirroring, or delivering the callback), so keep + // its report. + val step = if (mirrorBeforeCallback) "SAF mirroring failed" else "post-sync step failed" + native.copy( + completedAt = System.currentTimeMillis(), + success = false, + error = SyncStatus.normalizeError("$step: ${e.message}"), + ) + } else { + SyncStatus( + completedAt = System.currentTimeMillis(), + success = false, + error = SyncStatus.normalizeError("Exception: ${e.message}"), + ) + } + persistSyncStatus(status) handler.post { Log.e(TAG, "$operation failed", e) - callback(false, errorMsg) + callback(false, status.error ?: "sync failed") } } finally { executor.shutdown() @@ -222,6 +321,25 @@ class SyncInterface(context: Context) { } } + /** + * Persist the terminal status without letting a storage/broadcast failure + * escape. + * + * The completion callback is the caller's only signal that a pass ended — it + * is what clears [syncInFlight] in [syncBothAsync] and stops the UI waiting. + * If persistence threw inside [performSyncAsync]'s try (or, worse, inside its + * catch), the callback would never be posted and every later sync would be + * rejected as "already in flight". Persistence failing is not the sync + * failing, so it is logged and the callback still fires. + */ + private fun persistSyncStatus(status: SyncStatus) { + try { + AWPreferences(appContext).setLastSyncStatus(status) + } catch (e: Exception) { + Log.e(TAG, "Failed to persist sync status", e) + } + } + private fun mirrorSyncFilesToSafDir() { copySyncFilesToSafDir() } diff --git a/mobile/src/main/java/net/activitywatch/android/SyncSettingsActivity.kt b/mobile/src/main/java/net/activitywatch/android/SyncSettingsActivity.kt index 19027085..dace8df5 100644 --- a/mobile/src/main/java/net/activitywatch/android/SyncSettingsActivity.kt +++ b/mobile/src/main/java/net/activitywatch/android/SyncSettingsActivity.kt @@ -27,14 +27,39 @@ internal fun formatSyncStatus(status: SyncStatus?, dateFormat: DateFormat): Stri if (status == null) return "Last sync: never" val whenText = dateFormat.format(Date(status.completedAt)) - if (status.success) return "Last sync succeeded at $whenText" - - val error = SyncStatus.normalizeError(status.error) - return if (error != null) { - "Last sync failed at $whenText: $error" + val headline = if (status.success) { + "Last sync succeeded at $whenText" } else { - "Last sync failed at $whenText" + val error = SyncStatus.normalizeError(status.error) + if (error != null) { + "Last sync failed at $whenText: $error" + } else { + "Last sync failed at $whenText" + } + } + // Runs recorded before the SyncReport crossed the JNI boundary carry no + // counts; showing "pulled 0, pushed 0" for them would invent an answer. + if (!status.hasReport) return headline + return "$headline\n${formatSyncDetail(status)}" +} + +/** + * The per-run facts line: what moved and which peers it came from. Without + * this, a pass that transferred nothing is indistinguishable from one that + * transferred everything — the failure mode of a boolean-only status. + */ +internal fun formatSyncDetail(status: SyncStatus): String { + val peers = status.peersImported + status.peersSkipped + status.peersFailed + val parts = mutableListOf("pulled ${status.eventsPulled}, pushed ${status.eventsPushed}") + if (peers > 0) { + var peerText = "peers ${status.peersImported}/$peers imported" + if (status.peersSkipped > 0) peerText += ", ${status.peersSkipped} skipped" + if (status.peersFailed > 0) peerText += ", ${status.peersFailed} failed" + parts += peerText } + val line = parts.joinToString(" · ") + if (status.warnings.isEmpty()) return line + return (listOf(line) + status.warnings).joinToString("\n") } class SyncSettingsActivity : AppCompatActivity() { diff --git a/mobile/src/test/java/net/activitywatch/android/SyncSettingsActivityTest.kt b/mobile/src/test/java/net/activitywatch/android/SyncSettingsActivityTest.kt index 10a7e9f7..3fe62f6c 100644 --- a/mobile/src/test/java/net/activitywatch/android/SyncSettingsActivityTest.kt +++ b/mobile/src/test/java/net/activitywatch/android/SyncSettingsActivityTest.kt @@ -80,4 +80,189 @@ class SyncSettingsActivityTest { val normalized = SyncStatus.normalizeError(raw) assertEquals(SyncStatus.MAX_ERROR_CHARS, normalized!!.length) } + + @Test + fun formatSyncStatus_omitsCountsWhenPayloadCarriedNoReport() { + // Pre-SyncReport payloads must render exactly as they did before. + assertEquals( + "Last sync succeeded at 2026-09-01 01:30", + formatSyncStatus( + SyncStatus(completedAt = 1_788_226_200_000L, success = true, hasReport = false), + dateFormat, + ), + ) + } + + @Test + fun formatSyncStatus_showsThatASuccessfulPassMovedNothing() { + // The whole point of #274: a no-op success must not read as "succeeded". + assertEquals( + "Last sync succeeded at 2026-09-01 01:30\npulled 0, pushed 0", + formatSyncStatus( + SyncStatus(completedAt = 1_788_226_200_000L, success = true, hasReport = true), + dateFormat, + ), + ) + } + + @Test + fun formatSyncStatus_reportsCountsAndPeerOutcomes() { + assertEquals( + "Last sync succeeded at 2026-09-01 01:30\n" + + "pulled 1200, pushed 3 · peers 2/4 imported, 1 skipped, 1 failed", + formatSyncStatus( + SyncStatus( + completedAt = 1_788_226_200_000L, + success = true, + hasReport = true, + eventsPulled = 1200, + eventsPushed = 3, + peersImported = 2, + peersSkipped = 1, + peersFailed = 1, + ), + dateFormat, + ), + ) + } + + @Test + fun formatSyncStatus_listsWarningsBelowTheCounts() { + assertEquals( + "Last sync succeeded at 2026-09-01 01:30\n" + + "pulled 0, pushed 0\n" + + "no readable peers in sync folder\n" + + "push aborted after pull failure", + formatSyncStatus( + SyncStatus( + completedAt = 1_788_226_200_000L, + success = true, + hasReport = true, + warnings = listOf( + "no readable peers in sync folder", + "push aborted after pull failure", + ), + ), + dateFormat, + ), + ) + } + + @Test + fun formatSyncStatus_showsCountsBesideAFailure() { + assertEquals( + "Last sync failed at 2026-09-01 01:30: push failed: no such host\n" + + "pulled 12, pushed 0 · peers 1/2 imported, 1 failed", + formatSyncStatus( + SyncStatus( + completedAt = 1_788_226_200_000L, + success = false, + error = "push failed: no such host", + hasReport = true, + eventsPulled = 12, + peersImported = 1, + peersFailed = 1, + ), + dateFormat, + ), + ) + } + + @Test + fun fromJniResponse_parsesSyncReport() { + val status = SyncStatus.fromJniResponse( + """ + { + "success": true, + "message": "Synced 1200 events in from 2/4 peers (1 skipped, 1 failed)", + "events_pulled": 1200, + "events_pushed": 3, + "peers_imported": 2, + "peers_skipped": 1, + "peers_failed": 1, + "warnings": ["push aborted after pull failure"] + } + """.trimIndent(), + completedAt = 1_788_226_200_000L, + ) + + assertEquals(true, status.success) + assertEquals(true, status.hasReport) + assertEquals(1200, status.eventsPulled) + assertEquals(3, status.eventsPushed) + assertEquals(2, status.peersImported) + assertEquals(1, status.peersSkipped) + assertEquals(1, status.peersFailed) + assertEquals(listOf("push aborted after pull failure"), status.warnings) + assertEquals(null, status.error) + } + + @Test + fun fromJniResponse_acceptsPayloadWithoutReportFields() { + val status = SyncStatus.fromJniResponse( + """{"success": true, "message": "Successfully pulled from all hosts"}""", + completedAt = 1_788_226_200_000L, + ) + + assertEquals(true, status.success) + assertEquals(false, status.hasReport) + assertEquals(0, status.eventsPulled) + } + + @Test + fun fromJniResponse_treatsErrorPayloadAsFailure() { + val status = SyncStatus.fromJniResponse( + """{"success": false, "error": "Sync pull failed: connection refused"}""", + completedAt = 1_788_226_200_000L, + ) + + assertEquals(false, status.success) + assertEquals("Sync pull failed: connection refused", status.error) + assertEquals(false, status.hasReport) + } + + @Test + fun fromJniResponse_neverThrowsOnUnreadableResponse() { + val status = SyncStatus.fromJniResponse( + "not json at all", + completedAt = 1_788_226_200_000L, + ) + + assertEquals(false, status.success) + assertEquals(true, status.error!!.startsWith("Unreadable sync response:")) + } + + @Test + fun fromJniResponse_fallsBackToGenericErrorWhenReasonMissing() { + val status = SyncStatus.fromJniResponse( + """{"success": false}""", + completedAt = 1_788_226_200_000L, + ) + + assertEquals("sync failed", status.error) + } + + @Test + fun fromJniResponse_capsAndNormalizesWarnings() { + val warnings = "\"first\\n warning\"," + + (2..6).joinToString(",") { "\"warning $it\"" } + val status = SyncStatus.fromJniResponse( + """{"success": true, "events_pulled": 0, "warnings": [$warnings]}""", + completedAt = 1_788_226_200_000L, + ) + + assertEquals(SyncStatus.MAX_WARNINGS, status.warnings.size) + assertEquals("first warning", status.warnings[0]) + } + + @Test + fun fromJniResponse_ignoresNegativeCounts() { + val status = SyncStatus.fromJniResponse( + """{"success": true, "events_pulled": -5, "events_pushed": 2}""", + completedAt = 1_788_226_200_000L, + ) + + assertEquals(0, status.eventsPulled) + assertEquals(2, status.eventsPushed) + } }