From ab59838adcc9346270e93590ad0165c2e334eaf3 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 01:15:20 +0200 Subject: [PATCH 1/2] fix: stop automatic profile deletion replay --- .../java/to/bitkit/repositories/PubkyRepo.kt | 24 ------ .../screens/profile/EditProfileViewModel.kt | 2 +- .../to/bitkit/repositories/PubkyRepoTest.kt | 81 ++++++++----------- .../profile/EditProfileViewModelTest.kt | 32 ++++---- .../next/profile-delete-retry.fixed.md | 1 + .../delete-profile-uncertain-response.xml | 17 ++++ 6 files changed, 68 insertions(+), 89 deletions(-) create mode 100644 changelog.d/next/profile-delete-retry.fixed.md create mode 100644 journeys/profile/delete-profile-uncertain-response.xml diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 8340349b42..e92362b4a5 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -1086,16 +1086,6 @@ class PubkyRepo @Inject constructor( } } - suspend fun deleteProfileWithSessionRetry(): Result = withContext(ioDispatcher) { - val initialResult = deleteProfile() - if (initialResult.isSuccess) return@withContext initialResult - - val refreshedSession = refreshSessionIfPossible().getOrDefault(false) - if (!refreshedSession) return@withContext initialResult - - deleteProfile() - } - suspend fun deleteProfile(): Result = runSuspendCatching { withContext(ioDispatcher) { requireNotNull(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)) { @@ -1812,20 +1802,6 @@ class PubkyRepo @Inject constructor( } } - suspend fun refreshSessionIfPossible(): Result = runSuspendCatching { - withContext(ioDispatcher) { - val storedSecretKeyHex = activeSecretKeyHex() ?: return@withContext false - - pubkyService.signIn(storedSecretKeyHex) - val publicKey = pubkyService.publicKeyFromSecret(storedSecretKeyHex).ensurePubkyPrefix() - - notifyBackupStateChanged() - continueSignIn(publicKey) - - true - } - } - // endregion // region Sign out diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt index e2628806cf..06bb4bd4c5 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt @@ -273,7 +273,7 @@ class EditProfileViewModel @Inject constructor( try { privatePaykitRepo.beginProfileDeletion() privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true) - val result = pubkyRepo.deleteProfileWithSessionRetry() + val result = pubkyRepo.deleteProfile() if (result.isSuccess) { privatePaykitRepo.closeAndClear() ToastEventBus.send( diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index a6d429a882..28ad1a79a0 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1375,36 +1375,46 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `deleteProfileWithSessionRetry should refresh session and retry delete`() = test { - val expiredSession = "expired_session" - val newSession = "new_session" + fun `profile deletion does not replay after an uncertain response`() = test { val secretKey = "local_secret" - authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession) - whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession, newSession) + authenticateForTesting(publicKey = VALID_SELF_KEY, secret = "test_session") whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) - whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw TestAppError("Expired") }.thenReturn(Unit) - whenever(pubkyService.signIn(secretKey)).thenReturn(Unit) whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val uncertainResponse = TestAppError("Connection closed after profile deletion") + whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw uncertainResponse }.thenReturn(Unit) - val result = sut.deleteProfileWithSessionRetry() + val result = sut.deleteProfile() - assertTrue(result.isSuccess) - verifyBlocking(pubkyService, times(2)) { deletePaykitProfile() } + assertSame(uncertainResponse, result.exceptionOrNull()) + verify(pubkyService).deletePaykitProfile() + verify(pubkyService, never()).signIn(any()) + verify(pubkyService, never()).signOut() } @Test - fun `deleteProfileWithSessionRetry should return failure when session cannot refresh`() = test { - val expiredSession = "expired_session" - authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession) - whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession) - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null) - whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw TestAppError("Expired") } - - val result = sut.deleteProfileWithSessionRetry() + fun `failed profile deletion does not restore an identity after sign out`() = test { + val secretKey = "local_secret" + authenticateForTesting(publicKey = VALID_SELF_KEY, secret = "test_session") + whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) + whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val deletionStarted = CompletableDeferred() + val finishDeletion = CompletableDeferred() + val uncertainResponse = TestAppError("Connection closed after profile deletion") + whenever(pubkyService.deletePaykitProfile()).doSuspendableAnswer { + deletionStarted.complete(Unit) + withContext(NonCancellable) { finishDeletion.await() } + throw uncertainResponse + } + val deletion = async { sut.deleteProfile() } + deletionStarted.await() + sut.signOut().getOrThrow() + finishDeletion.complete(Unit) - assertTrue(result.isFailure) - verifyBlocking(pubkyService) { deletePaykitProfile() } - verifyBlocking(pubkyService, never()) { signIn(any()) } + assertSame(uncertainResponse, deletion.await().exceptionOrNull()) + assertNull(sut.publicKey.value) + assertFalse(sut.isAuthenticated.value) + verify(pubkyService, never()).signIn(any()) + verify(pubkyService).deletePaykitProfile() } @Test @@ -3466,31 +3476,6 @@ class PubkyRepoTest : BaseUnitTest() { verify(pubkyStore).reset() } - @Test - fun `refreshSessionIfPossible should refresh session when local secret key exists`() = test { - val secretKey = "local_secret" - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) - whenever(pubkyService.signIn(secretKey)).thenReturn(Unit) - whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) - - val result = sut.refreshSessionIfPossible() - - assertEquals(true, result.getOrNull()) - assertEquals(VALID_SELF_KEY, sut.publicKey.value) - assertTrue(sut.isAuthenticated.value) - } - - @Test - fun `refreshSessionIfPossible should return false when local secret key is missing`() = test { - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null) - - val result = sut.refreshSessionIfPossible() - - assertEquals(false, result.getOrNull()) - assertNull(sut.publicKey.value) - assertFalse(sut.isAuthenticated.value) - } - @Test fun `restoreSessionBackupState should derive local secret key for local seed backups`() = test { whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn("test mnemonic") @@ -4394,7 +4379,7 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `a contact edit started before the session of its identity is refreshed still saves`() = test { + fun `a contact edit started before the session of its identity is restored still saves`() = test { val store = stubGatedPubkyStore() authenticateForTesting(publicKey = VALID_SELF_KEY) whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn("local_secret") @@ -4402,7 +4387,7 @@ class PubkyRepoTest : BaseUnitTest() { whenever(pubkyService.publicKeyFromSecret("local_secret")).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) val signIn = checkNotNull(sut.currentSignIn()) - assertEquals(true, sut.refreshSessionIfPossible().getOrNull()) + sut.initialize().getOrThrow() val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) assertTrue(result.isSuccess) diff --git a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt index f9c005d101..381c03e5e4 100644 --- a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt @@ -60,7 +60,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.showDeleteConfirmation() advanceUntilIdle() verify(pubkyRepo, times(1)).forgetUnrestoredIdentity() - verify(pubkyRepo, never()).deleteProfileWithSessionRetry() + verify(pubkyRepo, never()).deleteProfile() verify(pubkyRepo, never()).saveProfile(any(), any(), any(), any(), any()) assertFalse(sut.uiState.value.showDeleteDialog) @@ -113,7 +113,7 @@ class EditProfileViewModelTest : BaseUnitTest() { val sut = createSut() advanceUntilIdle() val result = CompletableDeferred>() - whenever(pubkyRepo.deleteProfileWithSessionRetry()).doSuspendableAnswer { result.await() } + whenever(pubkyRepo.deleteProfile()).doSuspendableAnswer { result.await() } sut.showDeleteConfirmation() sut.deleteProfile() @@ -129,7 +129,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.disconnectProfile() sut.showDeleteConfirmation() advanceUntilIdle() - verify(pubkyRepo, times(1)).deleteProfileWithSessionRetry() + verify(pubkyRepo, times(1)).deleteProfile() verify(pubkyRepo, never()).saveProfile(any(), any(), any(), any(), any()) verify(pubkyRepo, never()).signOut() assertFalse(sut.uiState.value.showDeleteDialog) @@ -161,7 +161,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.showDeleteConfirmation() advanceUntilIdle() verify(pubkyRepo, times(1)).saveProfile(any(), any(), any(), any(), any()) - verify(pubkyRepo, never()).deleteProfileWithSessionRetry() + verify(pubkyRepo, never()).deleteProfile() verify(pubkyRepo, never()).forgetUnrestoredIdentity() assertFalse(sut.uiState.value.showDeleteDialog) @@ -183,7 +183,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `deleteProfile should emit success when repository delete succeeds`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -197,14 +197,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `profile created in Bitkit keeps delete copy`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -218,7 +218,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `ring identity is unlinked with the same cleanup`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut(isRingIdentity = true) advanceUntilIdle() @@ -234,7 +234,7 @@ class EditProfileViewModelTest : BaseUnitTest() { verify(context, never()).getString(R.string.profile__delete_success) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @@ -256,7 +256,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `retryDeleteProfile should emit success when repository delete succeeds`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -270,14 +270,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `deleteProfile should show retry dialog when delete still fails`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) @@ -295,7 +295,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `disconnectProfile should emit disconnect success`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) whenever(pubkyRepo.signOut()).thenReturn(Result.success(Unit)) @@ -354,7 +354,7 @@ class EditProfileViewModelTest : BaseUnitTest() { val sut = createSut() whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) } .thenReturn(Result.failure(TestAppError("cleanup failed"))) - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) advanceUntilIdle() sut.effects.test { @@ -367,14 +367,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.isDeleting) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `dismissDeleteFailureDialog should hide retry dialog`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) diff --git a/changelog.d/next/profile-delete-retry.fixed.md b/changelog.d/next/profile-delete-retry.fixed.md new file mode 100644 index 0000000000..fb97b9ecd4 --- /dev/null +++ b/changelog.d/next/profile-delete-retry.fixed.md @@ -0,0 +1 @@ +Prevent uncertain profile deletion failures from automatically repeating contact cleanup and deletion. diff --git a/journeys/profile/delete-profile-uncertain-response.xml b/journeys/profile/delete-profile-uncertain-response.xml new file mode 100644 index 0000000000..fe825c36ff --- /dev/null +++ b/journeys/profile/delete-profile-uncertain-response.xml @@ -0,0 +1,17 @@ + + + Requires a disposable test identity and a controlled homeserver proxy; never use a personal profile. + Configure the proxy to accept one profile deletion but lose its acknowledgement, retaining a redacted + request counter. This is a fault-injection fixture requirement, not evidence that this journey has run. + Normal SDK recovery may retry an individual request only after a confirmed authentication rejection; + the app must not restart contact cleanup and profile deletion because a request has an uncertain outcome. + + + Open Edit Profile and confirm deletion + Drop the acknowledgement after the server accepts the profile deletion + Verify failure options are shown and no success notification or onboarding navigation occurs + Wait through the network timeout and verify the app starts no second contact cleanup, profile deletion or sign-in + Cancel the failure dialog and verify no mutation is retried + Restart the app without choosing Retry and verify it does not automatically repeat the deletion + + From 99b446a59634e66ab8cf196ff0c109aa5bc9e070 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 01:16:43 +0200 Subject: [PATCH 2/2] docs: associate deletion fix with PR 1688 --- changelog.d/next/{profile-delete-retry.fixed.md => 1688.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{profile-delete-retry.fixed.md => 1688.fixed.md} (100%) diff --git a/changelog.d/next/profile-delete-retry.fixed.md b/changelog.d/next/1688.fixed.md similarity index 100% rename from changelog.d/next/profile-delete-retry.fixed.md rename to changelog.d/next/1688.fixed.md