From 03e7d172ee74f88ec30597e41327e03550ce68af Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Thu, 13 Aug 2026 10:10:01 +0200 Subject: [PATCH 1/4] file upload worker retry policy Signed-off-by: alperozturk96 --- .../client/jobs/BackgroundJobManagerImpl.kt | 6 ++++++ .../nextcloud/client/jobs/upload/FileUploadWorker.kt | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt index 4a28b1715dea..b565f9f50b75 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt @@ -117,6 +117,7 @@ internal class BackgroundJobManagerImpl( const val OFFLINE_OPERATIONS_PERIODIC_JOB_INTERVAL_MINUTES = 5L const val DEFAULT_IMMEDIATE_JOB_DELAY_SEC = 3L const val DEFAULT_BACKOFF_CRITERIA_DELAY_SEC = 300L + const val UPLOAD_BACKOFF_CRITERIA_DELAY_SEC = 45L private const val KEEP_LOG_MILLIS = 1000 * 60 * 60 * 24 * 3L @@ -655,6 +656,11 @@ internal class BackgroundJobManagerImpl( .addTag(tag) .setInputData(dataBuilder.build()) .setConstraints(constraints) + .setBackoffCriteria( + BackoffPolicy.LINEAR, + UPLOAD_BACKOFF_CRITERIA_DELAY_SEC, + TimeUnit.SECONDS + ) .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) .build() } diff --git a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt index ca8c9144e351..00dc7f691f10 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt @@ -47,6 +47,7 @@ import com.owncloud.android.operations.UploadFileOperation import com.owncloud.android.operations.factory.UploadFileOperationFactory import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.theme.ViewThemeUtils +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive @@ -153,7 +154,7 @@ class FileUploadWorker( Log_OC.e(TAG, "exception $t") activeOperations.values.forEach { it.cancel(null) } activeOperations.clear() - Result.failure() + retryIfStopped(t) } finally { // Ensure all database operations are complete before signaling completion uploadsStorageManager.notifyObserversNow() @@ -161,6 +162,13 @@ class FileUploadWorker( retryPolicy.reset() } + private fun retryIfStopped(t: Throwable): Result = if (t is CancellationException) { + Log_OC.d(TAG, "worker stopped, requesting retry") + Result.retry() + } else { + Result.failure() + } + private suspend fun trySetForeground() { try { val notificationTitle = notificationManager.currentOperationTitle @@ -286,7 +294,7 @@ class FileUploadWorker( if (canExitEarly()) { notificationManager.showConnectionErrorNotification() - return@withContext Result.failure() + return@withContext Result.retry() } fileUploadEventBroadcaster.sendUploadEnqueued(context) From 7abdc65d023d9732abc1c6a5c3e17367ef21e044 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Thu, 13 Aug 2026 10:12:26 +0200 Subject: [PATCH 2/4] docs(agents): tighten comment guidance for AI contributions Replace the single-line comment rule with a dedicated Comments section stating that names carry the explanation, when a comment is warranted, and an explicit never-list. Comments must not narrate the change or reference the issue, PR or the conversation that produced it. Apply the same rule to the java-to-kotlin skill conventions. Assisted-by: ClaudeCode:claude-opus-5 --- .../references/PROJECT-CONVENTIONS.md | 10 +++++-- AGENTS.md | 26 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.claude/skills/android-java-to-kotlin/references/PROJECT-CONVENTIONS.md b/.claude/skills/android-java-to-kotlin/references/PROJECT-CONVENTIONS.md index 5d2e15f89d00..be6fe9f38fd2 100644 --- a/.claude/skills/android-java-to-kotlin/references/PROJECT-CONVENTIONS.md +++ b/.claude/skills/android-java-to-kotlin/references/PROJECT-CONVENTIONS.md @@ -44,8 +44,14 @@ asks for; default to the "Nextcloud GmbH and Nextcloud contributors" line. - No decorative divider comments (`// ==== ====`, `// ---- Title ----`). `// region` / `// endregion` for IDE folding is allowed and should match the file's existing style. -- Prefer self-explanatory names over per-function KDoc. Preserve genuinely informative - Javadoc as KDoc (invariant 4); drop noise. +- Names carry the explanation, not comments. If a name needs a comment to be understood, + rename or decompose instead. Do not add KDoc to a function just because it is new. +- Preserve genuinely informative Javadoc as KDoc (invariant 4); drop noise. A comment + earns its place only for a non-obvious invariant, a platform/server workaround, or an + ordering requirement — and it explains **why**, never **what**. +- Never narrate the conversion itself: no "was a Java thread, now a coroutine", no + "changed from X to Y", no references to the PR, the issue, or this conversation. + Comments describe the code as it stands. That history belongs in the commit message. - Do not use multiple boolean flags to model state — use an `enum`/sealed class. ## Modern Java Interop diff --git a/AGENTS.md b/AGENTS.md index 9614463afedf..c7d2cee7611b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,9 +185,7 @@ All contributions generated or assisted by this agent must fully comply with: - Line length: **120 characters** - Standard Android Studio formatter with EditorConfig. - Kotlin preferred for new code; legacy Java still present. -- Do not use decorative section-divider comments of any kind (e.g. `// ── Title ───`, `// ------`, `// ======`). - Every new file must end with exactly one empty trailing line (no more, no less). -- Do not add comments, documentation for every function you created instead make it self explanatory as much as possible. - Create models, states in different files instead of doing it one single file. - Do not use magic numbers. - Apply fail fast principle instead of using nested if-else statements. @@ -195,3 +193,27 @@ All contributions generated or assisted by this agent must fully comply with: - Use modern Java for Java classes. Optionals, virtual threads, records, streams if necessary. - Avoid hardcoded strings, colors, dimensions. Use resources. - Run lint, spotbugsGplayDebug, detekt, spotlessKotlinCheck and fix findings inside the files that have been changed. + +## Comments + +Names carry the explanation, not comments. A function or variable whose purpose is not obvious from its +name is a naming problem or a decomposition problem — rename it or split it, do not describe it in a comment. +Assume the reader knows Kotlin, Java and the Android framework. + +Write a comment only when the code cannot carry the information itself: a non-obvious invariant, a workaround +for a platform or server bug, an ordering requirement, or a decision whose alternatives look equally valid and +are not. When you do write one, explain **why**, never **what**. + +Never write: + +- Comments that restate the code (`// increase the counter` above `counter++`). +- KDoc/Javadoc added to a function just because the function is new. +- Narration of the change or its history: "changed from X to Y", "previously this failed", "this is not a + permanent failure so we now retry", "fixes the crash reported in the issue". Comments describe the code as + it is today. A reader six months from now has no access to the issue, the PR discussion, or the conversation + that produced the change — that context belongs in the commit message and the PR description. +- Comments addressed at a reviewer or at the contributor rather than at a future reader. +- Decorative section-divider comments of any kind (e.g. `// ── Title ───`, `// ------`, `// ======`). + `// region` / `// endregion` for IDE folding is allowed where the file already uses it. + +Existing comments that are still accurate stay. Delete the ones the change makes wrong or obsolete. From f18ceee722e2e185eae270b865588814fd9d7b78 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Thu, 13 Aug 2026 10:19:34 +0200 Subject: [PATCH 3/4] wip Signed-off-by: alperozturk96 --- .../client/jobs/BackgroundJobManagerImpl.kt | 6 ------ .../nextcloud/client/jobs/upload/FileUploadWorker.kt | 12 ++---------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt index b565f9f50b75..4a28b1715dea 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt @@ -117,7 +117,6 @@ internal class BackgroundJobManagerImpl( const val OFFLINE_OPERATIONS_PERIODIC_JOB_INTERVAL_MINUTES = 5L const val DEFAULT_IMMEDIATE_JOB_DELAY_SEC = 3L const val DEFAULT_BACKOFF_CRITERIA_DELAY_SEC = 300L - const val UPLOAD_BACKOFF_CRITERIA_DELAY_SEC = 45L private const val KEEP_LOG_MILLIS = 1000 * 60 * 60 * 24 * 3L @@ -656,11 +655,6 @@ internal class BackgroundJobManagerImpl( .addTag(tag) .setInputData(dataBuilder.build()) .setConstraints(constraints) - .setBackoffCriteria( - BackoffPolicy.LINEAR, - UPLOAD_BACKOFF_CRITERIA_DELAY_SEC, - TimeUnit.SECONDS - ) .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) .build() } diff --git a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt index 00dc7f691f10..ca8c9144e351 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt @@ -47,7 +47,6 @@ import com.owncloud.android.operations.UploadFileOperation import com.owncloud.android.operations.factory.UploadFileOperationFactory import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.theme.ViewThemeUtils -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive @@ -154,7 +153,7 @@ class FileUploadWorker( Log_OC.e(TAG, "exception $t") activeOperations.values.forEach { it.cancel(null) } activeOperations.clear() - retryIfStopped(t) + Result.failure() } finally { // Ensure all database operations are complete before signaling completion uploadsStorageManager.notifyObserversNow() @@ -162,13 +161,6 @@ class FileUploadWorker( retryPolicy.reset() } - private fun retryIfStopped(t: Throwable): Result = if (t is CancellationException) { - Log_OC.d(TAG, "worker stopped, requesting retry") - Result.retry() - } else { - Result.failure() - } - private suspend fun trySetForeground() { try { val notificationTitle = notificationManager.currentOperationTitle @@ -294,7 +286,7 @@ class FileUploadWorker( if (canExitEarly()) { notificationManager.showConnectionErrorNotification() - return@withContext Result.retry() + return@withContext Result.failure() } fileUploadEventBroadcaster.sendUploadEnqueued(context) From a8a34e335ffb7d88b86e7e77c02424762b47fc8d Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Thu, 13 Aug 2026 10:19:53 +0200 Subject: [PATCH 4/4] wip Signed-off-by: alperozturk96 --- AGENTS.md | 1 - 1 file changed, 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index c7d2cee7611b..7cc05d89e677 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -212,7 +212,6 @@ Never write: permanent failure so we now retry", "fixes the crash reported in the issue". Comments describe the code as it is today. A reader six months from now has no access to the issue, the PR discussion, or the conversation that produced the change — that context belongs in the commit message and the PR description. -- Comments addressed at a reviewer or at the contributor rather than at a future reader. - Decorative section-divider comments of any kind (e.g. `// ── Title ───`, `// ------`, `// ======`). `// region` / `// endregion` for IDE folding is allowed where the file already uses it.