From 8e7ee3fae81ac3ead92787bffec7f2ac10844255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Acharn=C3=A9?= Date: Sun, 2 Aug 2026 18:19:56 +0700 Subject: [PATCH 1/2] fix(android): scope sync revocation lifecycle --- .../com/databreeze/android/AndroidRuntime.kt | 11 +++++++ .../com/databreeze/android/sync/SyncPorts.kt | 30 +++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/apps/android/app/src/main/java/com/databreeze/android/AndroidRuntime.kt b/apps/android/app/src/main/java/com/databreeze/android/AndroidRuntime.kt index 003172d4..77686bb6 100644 --- a/apps/android/app/src/main/java/com/databreeze/android/AndroidRuntime.kt +++ b/apps/android/app/src/main/java/com/databreeze/android/AndroidRuntime.kt @@ -2,6 +2,7 @@ package com.databreeze.android import android.content.Context import com.databreeze.android.security.AndroidDeviceKeyStore +import com.databreeze.android.security.DeviceKeyHandle import com.databreeze.android.security.DeviceKeyStore import com.databreeze.android.storage.LocalStorePort import com.databreeze.android.storage.AccountWorkspaceScope @@ -23,6 +24,16 @@ class AndroidRuntime private constructor( val syncRevocationGuard: SyncRevocationGuard, val workerFactory: DataBreezeWorkerFactory, ) { + /** + * Re-enables a scope only after authentication has succeeded and its device key is ready. + * The explicit call prevents a process restart from silently undoing sign-out revocation. + */ + suspend fun signIn(scope: AccountWorkspaceScope, keyAlias: String): DeviceKeyHandle { + val handle = deviceKeyStore.getOrCreate(keyAlias) + syncRevocationGuard.reactivate(scope) + return handle + } + /** Revocation/account switch clears local work and the device-bound key before returning. */ suspend fun signOut(scope: AccountWorkspaceScope, keyAlias: String) { syncRevocationGuard.revoke(scope) diff --git a/apps/android/app/src/main/java/com/databreeze/android/sync/SyncPorts.kt b/apps/android/app/src/main/java/com/databreeze/android/sync/SyncPorts.kt index a89649c9..f0d8f63c 100644 --- a/apps/android/app/src/main/java/com/databreeze/android/sync/SyncPorts.kt +++ b/apps/android/app/src/main/java/com/databreeze/android/sync/SyncPorts.kt @@ -86,17 +86,18 @@ interface SyncTransport { interface SyncRevocationGuard { suspend fun withPermit(scope: AccountWorkspaceScope, operation: suspend () -> T): T? suspend fun revoke(scope: AccountWorkspaceScope) + suspend fun reactivate(scope: AccountWorkspaceScope) } class SharedPreferencesSyncRevocationGuard( private val preferences: SharedPreferences, ) : SyncRevocationGuard { - private val mutex = Mutex() + private val mutexes = java.util.concurrent.ConcurrentHashMap() override suspend fun withPermit( scope: AccountWorkspaceScope, operation: suspend () -> T, - ): T? = mutex.withLock { + ): T? = mutexFor(scope).withLock { if (preferences.getBoolean(key(scope), false)) null else operation() } @@ -104,28 +105,45 @@ class SharedPreferencesSyncRevocationGuard( check(preferences.edit().putBoolean(key(scope), true).commit()) { "unable to persist sync revocation" } - mutex.withLock { Unit } + mutexFor(scope).withLock { Unit } + } + + override suspend fun reactivate(scope: AccountWorkspaceScope) { + mutexFor(scope).withLock { + check(preferences.edit().remove(key(scope)).commit()) { + "unable to persist sync reactivation" + } + } } private fun key(scope: AccountWorkspaceScope): String = "revoked-${scope.stableKey}" + private fun mutexFor(scope: AccountWorkspaceScope): Mutex = + mutexes.computeIfAbsent(scope.stableKey) { Mutex() } } /** Deterministic guard used by JVM tests and dependency-free shell configurations. */ class InMemorySyncRevocationGuard : SyncRevocationGuard { - private val mutex = Mutex() + private val mutexes = java.util.concurrent.ConcurrentHashMap() private val revoked = java.util.concurrent.ConcurrentHashMap.newKeySet() override suspend fun withPermit( scope: AccountWorkspaceScope, operation: suspend () -> T, - ): T? = mutex.withLock { + ): T? = mutexFor(scope).withLock { if (scope.stableKey in revoked) null else operation() } override suspend fun revoke(scope: AccountWorkspaceScope) { revoked += scope.stableKey - mutex.withLock { Unit } + mutexFor(scope).withLock { Unit } } + + override suspend fun reactivate(scope: AccountWorkspaceScope) { + mutexFor(scope).withLock { revoked.remove(scope.stableKey) } + } + + private fun mutexFor(scope: AccountWorkspaceScope): Mutex = + mutexes.computeIfAbsent(scope.stableKey) { Mutex() } } interface SyncScheduler { From c679cbba957582dde1edd892d22e59c9b16cc0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Acharn=C3=A9?= Date: Sun, 2 Aug 2026 18:20:00 +0700 Subject: [PATCH 2/2] test(android): verify reactivation and scope isolation --- .../android/SyncSchedulerContractTest.kt | 34 +++++++++++++++++++ docs/operations/code-review-14-disposition.md | 11 ++++++ tools/repo-cli/test/android-shell.test.mjs | 3 ++ 3 files changed, 48 insertions(+) diff --git a/apps/android/app/src/test/java/com/databreeze/android/SyncSchedulerContractTest.kt b/apps/android/app/src/test/java/com/databreeze/android/SyncSchedulerContractTest.kt index 4be3f868..f97d4508 100644 --- a/apps/android/app/src/test/java/com/databreeze/android/SyncSchedulerContractTest.kt +++ b/apps/android/app/src/test/java/com/databreeze/android/SyncSchedulerContractTest.kt @@ -7,6 +7,7 @@ import com.databreeze.android.sync.WorkManagerSyncScheduler import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.async import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Test @@ -46,4 +47,37 @@ class SyncSchedulerContractTest { revocation.await() assertNull(guard.withPermit(scope) { "must-not-send" }) } + + @Test + fun reactivation_restores_sync_after_an_explicit_sign_in() = runBlocking { + val guard = InMemorySyncRevocationGuard() + val scope = AccountWorkspaceScope("account-1", "workspace-1") + + guard.revoke(scope) + assertNull(guard.withPermit(scope) { "blocked" }) + guard.reactivate(scope) + + assertEquals("allowed", guard.withPermit(scope) { "allowed" }) + } + + @Test + fun revocation_for_one_scope_does_not_wait_for_another_scope() = runBlocking { + val guard = InMemorySyncRevocationGuard() + val first = AccountWorkspaceScope("account-1", "workspace-1") + val second = AccountWorkspaceScope("account-2", "workspace-2") + val entered = CompletableDeferred() + val release = CompletableDeferred() + + val inFlight = async { + guard.withPermit(first) { + entered.complete(Unit) + release.await() + } + } + entered.await() + withTimeout(1_000) { guard.revoke(second) } + release.complete(Unit) + inFlight.await() + Unit + } } diff --git a/docs/operations/code-review-14-disposition.md b/docs/operations/code-review-14-disposition.md index 066d2792..dfeada9e 100644 --- a/docs/operations/code-review-14-disposition.md +++ b/docs/operations/code-review-14-disposition.md @@ -35,3 +35,14 @@ review branch: All actionable findings were handled in focused commits. Hosted checks and the connected Android tests remain required before merging the review-fix PR and the synchronized promotion PR. + +## Automatic incremental review after PR #15 synchronization + +GitHub automatically refreshed the existing review when PR #15 merged into `dev` (no new review +was requested). It identified two additional valid claims, both fixed in the follow-up branch: + +- Revocation locks are now keyed by `AccountWorkspaceScope`, so a long-running sync in one + workspace cannot stall sign-out in another. +- The guard now exposes `reactivate(scope)` and `AndroidRuntime.signIn` calls it only after the + device key is initialized, allowing an explicit sign-in/sign-out cycle without silently + re-enabling a revoked scope on process restart. diff --git a/tools/repo-cli/test/android-shell.test.mjs b/tools/repo-cli/test/android-shell.test.mjs index 82c956a0..ef38c6d5 100644 --- a/tools/repo-cli/test/android-shell.test.mjs +++ b/tools/repo-cli/test/android-shell.test.mjs @@ -75,13 +75,16 @@ test('Android shell has durable local state, injected workers, and process-death const localStore = read('app/src/main/java/com/databreeze/android/storage/LocalStore.kt'); const sync = read('app/src/main/java/com/databreeze/android/sync/SyncPorts.kt'); const app = read('app/src/main/java/com/databreeze/android/DataBreezeApplication.kt'); + const runtime = read('app/src/main/java/com/databreeze/android/AndroidRuntime.kt'); assert.match(localStore, /@Database\(entities = \[SyncQueueEntity::class\]/u); assert.match(localStore, /primaryKeys = \["accountId", "workspaceId", "mutationId"\]/u); assert.match(sync, /ExistingWorkPolicy\.APPEND_OR_REPLACE/u); + assert.match(sync, /suspend fun reactivate\(scope: AccountWorkspaceScope\)/u); assert.match(sync, /DataBreezeWorkerFactory/u); assert.match(sync, /setRequiredNetworkType\(NetworkType\.CONNECTED\)/u); assert.match(app, /Configuration\.Provider/u); assert.match(app, /setWorkerFactory\(runtime\.workerFactory\)/u); + assert.match(runtime, /suspend fun signIn\(scope: AccountWorkspaceScope/u); assert.ok( existsSync( path.join(