Repository navigation
fix: stop deletion after incomplete cleanup - #1690
ben-kaufman wants to merge 8 commits into
Conversation
 [Medium impact] This PR appears safe to merge within its stated cleanup scope. SummaryThis PR stops profile deletion when private payment-link withdrawal or contact removal is incomplete.
No actionable issues were found. Tests were inspected but not run. ben-kaufman explicitly deferred session ownership, manual-retry binding, restart reconciliation, and cross-device conditional removal. This assessment does not claim those limitations are resolved. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Confirm profile deletion] --> B[Withdraw private payment endpoints]
B -->|Failure| F[Show deletion failure options]
B -->|Success| C[Read and remove contacts]
C -->|Read error or uncertain result| F
C -->|Partial removal| D[Keep unacknowledged contact data]
D --> F
C -->|Complete| E[Delete profile and sign out]
E -->|Failure| F
E -->|Success| G[Clear local private payment state]
G -->|Failure| H[Show error without success or deletion retry]
G -->|Success| I[Report success and leave profile]
Reviews (1) · Last reviewed commit: "fix: stop profile deletion after incompl..." · Reviewed by Greptile |
|
Triage: 3.1.0. It fixes shipped profile deletion, and 3.1.0 is due 28 Oct. |
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Profile deletion now stops when private endpoint withdrawal, the contact read or the contact removal is incomplete, keeps the contacts and overrides that were not acknowledged as removed, and no longer reports success when local finalization fails. Reviewed 479c530, full tier, stacked on #1688 (fix/profile-delete-session-recovery), so the delta against that base is what I read.
What I checked, and 6 candidates I ruled out
Read in full: PubkyRepo.deleteProfile / deleteAllContacts / signOut / clearAuthenticatedState, EditProfileViewModel.attemptDeleteProfile / disconnectProfile, PrivatePaykitRepo.removePublishedEndpointsForCleanup / removePublishedEndpointsLocked / closeAndClear, PaykitSdkService.contactRecords / removeContacts, and remove_contacts_and_block_peers / contact_records in paykit-rs at v0.1.1-rc1 (the pinned paykit version).
Call sites traced: deleteAllContacts (only deleteProfile), removePublishedEndpointsForCleanup(..., deferAppSync = true) (only the delete flow), closeAndClear (delete and disconnect).
CI: build-local green; build, lint and the e2e matrix still running when this was posted.
Ruled out
- A missing-storage read now blocks deletion: dropping the
isMissingPubkyData()fallback is safe becausecontact_recordsis a local SDK storage transaction; it fails only on an uninitialized identity or a storage error, neither of which is evidence of an empty contact list. - Overrides of local-only contacts survive a full success: the old code cleared them to
emptyMap(), the new code subtracts onlyremovedKeys; on successsignOut()reachesclearAuthenticatedState, whosepubkyStore.reset()drops them anyway. - Key format mismatch between records, overrides and
_contacts: all three are compared afterensurePubkyPrefix(), and override keys are written prefixed (PubkyRepo.kt:2054); the new partial-removal test exercises the subtraction. - Contacts the SDK retains by design (active recurring request, peer lease, published marker) now make deletion fail on every retry: this is the stated intent of the PR and its journey (partial removal preserves contacts and prevents deletion), not an accident.
- User left on Edit Profile after
closeAndClearfails post-deletion: deliberate per the newfailed local finalization does not report deletion successtest; the failure dialog is not shown because the remote deletion did happen. - Endpoint withdrawal failure leaves the deletion flag set:
endProfileDeletion()stays in thefinallyunderNonCancellable, andremovePublishedEndpointsForCleanupmarkscontactSharingCleanupPendingon failure.
Not built here: on the Bitkit track an auto review does not run a PR branch's Gradle build, so this is reasoned from code and CI.
Merge confidence: 4/5, no findings, but the change is reasoned from code rather than built here, and build, lint and e2e had not finished.
Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: one LOW inline, reproduced with a failing unit test.
Checked and clean:
- When cleanup is incomplete, the user gets the existing Retry / Disconnect / Cancel dialog.
- Retry is idempotent after a late
signOutfailure, becausefetch_paykit_profilereturnsNonefor an absent profile, so dropping the 404 tolerance is safe. - No path locks the user out of deletion or sign-in for good.
- Private endpoints are now withdrawn before the profile delete, which closes a gap on master.
- With #1688 removing the automatic replay, each stop is final until a manual Retry; the two don't conflict.
profile-cleanup-failure.xml's fault point is right: contact removal goes through the shared-state storage adapter (PubkyShared,put_locked), so a homeserver proxy can fail it.- Twin synonymdev/bitkit-ios#1124 clears the cleanup flag on a successful withdrawal, so it doesn't have the LOW.
Device gate (partial): 2daec90 — not run yet. profile-cleanup-failure.xml needs a homeserver fault proxy. The partial-removal stop can be driven without one, using a contact with an active recurring payment request on two devices: deletion stops, the dialog shows, and after Cancel the contact can or can't pay privately. Device results will follow in a separate review.
| privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true) | ||
| val result = pubkyRepo.deleteProfile() | ||
| val result = runSuspendCatching { | ||
| privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true).getOrThrow() |
There was a problem hiding this comment.
When deletion stops after this deferred withdrawal succeeded, private endpoints stay withdrawn until the next foreground or network event. With deferAppSync = true, a successful withdrawal leaves cleanupPending = true (PrivatePaykitRepo.kt:508-513). scheduleContactPreparation returns early while it's set. endProfileDeletion() only flips the in-progress flag, and AppViewModel's follow-up synchronizePaykitContacts → scheduleSavedContactPreparation also does nothing. Only retryPendingEndpointRemoval clears the flag, and it runs on foreground (ContentView.kt:319 → AppViewModel.kt:871), network restore, channel events or a USDT availability change.
So after Cancel on the failure dialog the user is signed in, but their contacts can't pay them privately until the app is backgrounded and reopened. Master had the same deferred flag only on the delete-fails branch. This PR adds three stop cases that now reach it: a partial contact removal, a contact-read error, and a 404 from the profile delete. iOS clears the flag on any successful withdrawal (PubkyProfileManager.swift:1498).
Fix: keep the flag (the crash-recovery test interrupted profile deletion retries registry sync from persisted cleanup state depends on it) and reconcile on the stop path instead, in this function's finally:
withContext(NonCancellable) {
privatePaykitRepo.endProfileDeletion()
if (_uiState.value.showDeleteFailureDialog) {
privatePaykitRepo.retryPendingEndpointRemoval(pubkyRepo.contacts.value.map { it.publicKey })
privatePaykitRepo.refreshKnownSavedContactEndpoints(TAG)
}
}In production, route this through contactPaymentSettingsRepo.reconcilePendingEndpoints or an equivalent entry point, as the foreground path does.
LOW.
Reproduction
EditProfileViewModelTest > stopped deletion reconciles withdrawn private endpoints fails at 2daec90 and passes with the fix (20/20):
Wanted but not invoked:
privatePaykitRepo.retryPendingEndpointRemoval(...)
PrivatePaykitRepoTest > stopped profile deletion keeps private endpoints withdrawn until pending cleanup is retried passes at head by design. It shows the stuck state against the real repo: no ensureLinkWithPeer after preparation is rescheduled, with the flag still set. A retry plus a refresh republishes.
./gradlew testDevDebugUnitTest --tests 'to.bitkit.ui.screens.profile.EditProfileViewModelTest' --tests 'to.bitkit.repositories.PrivatePaykitRepoTest'
diff --git a/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt
index 5e0aa48fe..cb5ef2438 100644
--- a/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt
+++ b/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt
@@ -370,6 +370,38 @@ class PrivatePaykitRepoTest : BaseUnitTest(StandardTestDispatcher()) {
sut.closeAndClear()
}
+ @Test
+ fun `stopped profile deletion keeps private endpoints withdrawn until pending cleanup is retried`() = test {
+ settingsData.update {
+ SettingsData(
+ sharesPrivatePaykitEndpoints = true,
+ publicPaykitLightningEnabled = false,
+ publicPaykitUsdtEnabled = false
+ )
+ }
+ cacheData.update { it.copy(contacts = mapOf(CONTACT_KEY to cachedPublishedContact())) }
+ sut = createSut()
+
+ sut.beginProfileDeletion()
+ sut.removePublishedEndpointsForCleanup("profile deletion", deferAppSync = true).getOrThrow()
+ sut.endProfileDeletion()
+ sut.scheduleSavedContactPreparation(listOf(CONTACT_KEY)).getOrThrow()
+ runCurrent()
+ sut.awaitContactPreparation()
+
+ assertTrue(cacheData.value.cleanupPending)
+ verify(paykitSdkService, never()).ensureLinkWithPeer(CONTACT_KEY)
+
+ sut.retryPendingEndpointRemoval(listOf(CONTACT_KEY)).getOrThrow()
+ sut.refreshKnownSavedContactEndpoints("profile deletion stopped").getOrThrow()
+ runCurrent()
+ sut.awaitContactPreparation()
+
+ assertFalse(cacheData.value.cleanupPending)
+ verify(paykitSdkService).ensureLinkWithPeer(CONTACT_KEY)
+ sut.closeAndClear()
+ }
+
@Test
fun `handleOnchainActivity skips SDK access when publication is locally unnecessary`() = test {
settingsData.value = SettingsData(
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 30f48762a..eda68d8ea 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
@@ -12,6 +12,7 @@ import kotlinx.coroutines.test.advanceUntilIdle
import org.junit.Test
import org.mockito.Mockito.clearInvocations
import org.mockito.kotlin.any
+import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.doSuspendableAnswer
import org.mockito.kotlin.eq
import org.mockito.kotlin.inOrder
@@ -40,6 +41,7 @@ class EditProfileViewModelTest : BaseUnitTest() {
private val context: Context = mock()
private val pubkyRepo: PubkyRepo = mock()
private val privatePaykitRepo: PrivatePaykitRepo = mock()
+ private val contacts = MutableStateFlow<List<PubkyProfile>>(emptyList())
@Test
fun `disconnect finishes private cleanup after the screen is closed`() = test {
@@ -318,6 +320,28 @@ class EditProfileViewModelTest : BaseUnitTest() {
}
}
+ @Test
+ fun `stopped deletion reconciles withdrawn private endpoints`() = test {
+ whenever(pubkyRepo.deleteProfile()).thenReturn(Result.failure(TestAppError("contact retained")))
+ contacts.value = listOf(createProfile())
+ whenever { privatePaykitRepo.retryPendingEndpointRemoval(any()) }.thenReturn(Result.success(Unit))
+ whenever { privatePaykitRepo.refreshKnownSavedContactEndpoints(any(), any(), any(), anyOrNull()) }
+ .thenReturn(Result.success(Unit))
+ val sut = createSut()
+ advanceUntilIdle()
+
+ sut.deleteProfile()
+ advanceUntilIdle()
+
+ assertTrue(sut.uiState.value.showDeleteFailureDialog)
+ inOrder(privatePaykitRepo).apply {
+ verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true))
+ verify(privatePaykitRepo).endProfileDeletion()
+ verify(privatePaykitRepo).retryPendingEndpointRemoval(listOf(TEST_PUBLIC_KEY))
+ verify(privatePaykitRepo).refreshKnownSavedContactEndpoints(any(), any(), any(), anyOrNull())
+ }
+ }
+
@Test
fun `deleteProfile should show retry dialog when delete still fails`() = test {
whenever(pubkyRepo.deleteProfile()).thenReturn(
@@ -426,6 +450,10 @@ class EditProfileViewModelTest : BaseUnitTest() {
whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) }
.thenReturn(Result.success(Unit))
whenever { privatePaykitRepo.closeAndClear() }.thenReturn(Result.success(Unit))
+ whenever(pubkyRepo.contacts).thenReturn(contacts)
+ whenever { privatePaykitRepo.retryPendingEndpointRemoval(any()) }.thenReturn(Result.success(Unit))
+ whenever { privatePaykitRepo.refreshKnownSavedContactEndpoints(any(), any(), any(), anyOrNull()) }
+ .thenReturn(Result.success(Unit))
return EditProfileViewModel(
context = context,There was a problem hiding this comment.
The retained cleanup flag and the missing immediate follow-up are real at this head. I am addressing this with the original-owner/restart repair tracked in #1687 rather than automatically republishing after an uncertain destructive operation. That candidate persists a removal obligation before cleanup, shows pending recovery, and offers read-only Review followed by a new confirmation or an explicitly local Disconnect. It deliberately keeps authenticated publication paused until the obligation is resolved. Its UI integration and controlled retry tests are under final validation; it is not published yet. I will link the follow-up and its evidence here. Leaving this thread open.
There was a problem hiding this comment.
The integrated recovery follow-up is now published in #1693 (stacked on this PR). It persists pending removal before destructive work, keeps authenticated publication paused after an uncertain outcome, and exposes read-only Review with a fresh original-owner confirmation or explicit local-only Disconnect. It deliberately does not republish automatically after a potentially completed deletion. Final 4,725 unit/component tests, full compile/format/lint and three installed Compose recovery cases passed; two consecutive independent final reviews are clean and both commits are signed/verified. Complete authenticated fault journeys and native contact CAS remain documented gaps in #1687. Leaving this thread open for review of that recovery behavior rather than claiming the parent alone restores publication safely.
There was a problem hiding this comment.
Still open at 815e85c. Fine to resolve it in #1693, since that PR keeps publication paused until Review or Disconnect. I'll check it there.
There was a problem hiding this comment.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: I found no additional defects. The existing publication-pause finding remains acknowledged at this head; the author's follow-up tracks its recovery separately.
QA for 2daec90: Android 16 / API 36, Core 0.8.0-rc11 and Paykit 0.1.1-rc1. Device cases used this head plus the test fixture below, with disposable identities and an unfunded regtest wallet. The real Edit Profile screen, ViewModel, repositories and native SDK handled deletion; a homeserver proxy supplied the faults. The installed APK hash matched the built fixture APK.
Test 1: Manual 1 / J1 — private cleanup fails ✅ Failure stops before contact or profile deletion.
The proxy returned 503 for the native shared-state read during private cleanup. After Cancel, the refreshed inspector confirmed the session, both contacts and overrides, and both sharing settings remained. Call counters and the proxy trace confirmed no contact or profile deletion.
1a.2x.mp4
Private cleanup failure and retained state, at 2× speed.
Test 2: Manual 1 / J1 — unconfirmed contact removal ✅ Local evidence survives without entering profile deletion.
The proxy accepted the native write-ahead record, then rejected shared-state publication with 503. The failure dialog appeared. The post-failure inspector and call trace confirmed both contacts and overrides remained, with no profile deletion or success feedback.
| Failure flow | Refreshed post-failure inspector |
|---|---|
1b.2x.mp4 |
 |
Unconfirmed cleanup at 2× speed; the still shows the refreshed counters after Cancel.
Test 3: Manual 1 / J1 — partial native removal ✅ Only the acknowledged contact and override disappear.
The SDK retained a contact with a published public marker and acknowledged the other removal. The inspector showed one remaining contact and override; the persisted override key matched the retained marker. Failure appeared without entering profile deletion. The marker was provisioned through the native SDK.
1c.2x.mp4
Partial removal and retained evidence, at 2× speed.
Test 4: Manual 2 — profile deletion returns 404 ✅ Failure preserves the session and sharing settings.
The proxy trace confirmed profile GET 200 followed by DELETE 404. The app showed failure and retained its session and both sharing settings, with no success feedback. Earlier acknowledged contact removals remained applied, as the refreshed inspector shows.
2.2x.mp4
Rejected profile deletion and retained session, at 2× speed.
Test 5: J1 — Cancel and cold restart ✅ Normal startup restores the same identity without replaying profile deletion.
After unconfirmed contact cleanup and Cancel, a cold launch through the normal app entry restored the same authenticated identity on Home and Profile. The profile QR identifier and restoration log confirmed the owner. Proxy sampling covered over 85 seconds, with successful profile reads and no profile DELETE.
restart.2x.mp4
Normal cold startup and restored profile, at 2× speed.
Check 1: unmodified-head build and regression checks ✅ The app build and affected suites pass.
assembleDevDebug and testDevDebugUnitTest with PubkyRepoTest and EditProfileViewModelTest passed on the unmodified head. git diff --check passed. These focused checks include unavailable contact reads, uncertain/partial removals, 404-like profile errors, private cleanup failure and failed local finalization.
Full review of 99b446a..2daec90, all six changed files and reached cleanup paths. Contact-read errors and local-finalization storage failures had source/unit coverage rather than separate device cases. Active-recurring and peer-lease retention variants were not separately driven. Manual Retry/Disconnect ownership and native cross-device conditional removal remain outside this PR's scope.
Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup matches the scoped cleanup/error propagation behavior at a062618; its authenticated device report supplies the iOS evidence.
Test fixture used by Tests 1–5
Not pushed. Provisions disposable native identities and contacts, displays call/state evidence, and renders the actual Edit Profile screen. Public-contact publication is enabled to stage the retained-marker variant. SDK calls return native results. Test 5 launches the normal app entry.
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..304e984d2 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
android:usesCleartextTraffic="true"
tools:ignore="MissingApplicationIcon">
+ <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" /> <!-- test hook -->
+
<provider
android:name="to.bitkit.dev.DevToolsProvider"
android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 87c452f64..011d57041 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
suspend fun fetchPubkyFollows(publicKey: String): List<String> =
publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
+ internal suspend fun reviewPublishContact(key: String) { // test hook: provision real native retained marker
+ isSetup.await() // test hook
+ operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } } // test hook
+ } // test hook
+
suspend fun contactRecords(): List<ContactRecord> {
isSetup.await()
return operationLock.withLock {
@@ -1952,7 +1957,7 @@ internal object BitkitPaykitSdkConfig {
}
internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
- publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+ publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: native public-marker fixture
)
internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index e9105a40c..560146420 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,6 +208,7 @@ class PubkyService @Inject constructor(
}
suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
paykitSdkService.deletePaykitProfile()
}
@@ -216,6 +217,7 @@ class PubkyService @Inject constructor(
}
suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.reads++; to.bitkit.dev.CleanupProbe.note("contact read entered") // test hook
paykitSdkService.contactRecords()
}
@@ -252,6 +254,7 @@ class PubkyService @Inject constructor(
}
suspend fun removeContacts(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("contact remove entered count=${publicKeys.size}") // test hook
paykitSdkService.removeContacts(publicKeys)
}
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..f0c70a077
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,113 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+ var reads = 0
+ var removals = 0
+ var deletes = 0
+ fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+ @Inject lateinit var service: PubkyService
+ @Inject lateinit var sdk: PaykitSdkService
+ @Inject lateinit var repo: PubkyRepo
+ @Inject lateinit var privateRepo: PrivatePaykitRepo
+ @Inject lateinit var pubkyStore: PubkyStore
+ @Inject lateinit var settings: SettingsStore
+ @Inject lateinit var keychain: Keychain
+ @Inject lateinit var wallet: WalletRepo
+ private var status by mutableStateOf("Preparing disposable native identity")
+ private var evidence by mutableStateOf("")
+ private var vm by mutableStateOf<EditProfileViewModel?>(null)
+ private var exited by mutableStateOf(false)
+ private var successToasts = 0 // test hook
+
+ override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+ setContent {
+ AppThemeSurface {
+ Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+ Text(status, Modifier.padding(horizontal = 12.dp))
+ Text(evidence, Modifier.padding(horizontal = 12.dp))
+ Row {
+ TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+ TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+ }
+ if (exited) Text("Deletion success navigation")
+ vm?.let { current ->
+ Box(Modifier.weight(1f)) {
+ EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+ }
+ }
+ }
+ }
+ }
+ lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+ lifecycleScope.launch {
+ try {
+ if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+ service.initialize()
+ if (intent.getBooleanExtra("resume", false)) {
+ repo.initialize().getOrThrow()
+ status = "Restored through normal profile initialization"
+ } else {
+ val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ val home = service.publicKeyFromSecret("00".repeat(32))
+ service.signUp(secret, home, null)
+ service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+ repo.initialize().getOrThrow()
+ val peers = listOf("11", "22").map { service.publicKeyFromSecret(it.repeat(32)) }
+ for ((index, peer) in peers.withIndex()) {
+ service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+ }
+ if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+ repo.loadContacts()
+ pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+ settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+ status = "Authenticated disposable identity"
+ }
+ CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0
+ vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+ CleanupProbe.note("READY owner=${repo.publicKey.value}")
+ inspect()
+ } catch (e: Exception) {
+ status = "Provisioning failed: ${e.message}"
+ CleanupProbe.note(status)
+ Log.e("CleanupProbe", "setup", e)
+ }
+ }
+ }
+
+ private suspend fun inspect() {
+ val state = settings.data.first()
+ val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+ val overrides = pubkyStore.data.first().contactProfileOverrides.size
+ evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts"
+ CleanupProbe.note(evidence)
+ }
+}Case 1a: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0
Case 1b: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Case 1c: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Case 2: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full review of the complete PR diff against its merge base 99b446a (the head of dependency #1688), at 2daec90. No earlier QA baseline existed. Coverage: PubkyRepo.deleteProfile / deleteAllContacts; the error paths in EditProfileViewModel for profile deletion and Disconnect; the callers PrivatePaykitRepo.removePublishedEndpointsForCleanup / closeAndClear, signOut / clearAuthenticatedState, and the PaykitSdkService contact and profile wrappers; the updated unit tests and the new journey. Also checked: the Paykit SDK contracts at paykit-rs v0.1.1-rc1 (f384694) and parity with iOS twin bitkit-ios#1124 at a062618.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The stricter stop-on-incomplete-cleanup behaviour looks sound for transient failures. Dropping the 404/"missing" tolerance is safe: in the SDK, fetch_paykit_profile returns None when the profile is already gone, so deletePaykitProfile completes normally, and contact_records is a local storage read. Contact keys are compared after normalising the pubky prefix in every place. Returning early when there are no contacts leaves no stale state, because the successful signOut() path calls pubkyStore.reset() and clears the contact list. One problem remains: contacts that the SDK keeps on purpose, because of an active recurring subscription, now block profile deletion permanently without saying why (see the inline finding). The iOS twin has the same behaviour: removeContacts skips subscribed contacts and the best-effort wrapper is gone, so the concern applies to both platforms.
Existing unresolved thread: deferred withdrawal leaves private endpoints withdrawn after a stopped deletion. The author says it is tracked in #1687 / #1693. I did not re-report it here.
Validation: no build or tests were run in this review. The author's CI at this head passed: CI, Lint, E2E. None of these exercises the new cleanup-fault journey.
Device testing: not performed in this review.
Suggested additional test cases
- Android, two wallets with different Pubky identities (A and B), plus a third saved contact C on A. A saves B and C as contacts and holds an active recurring subscription with B, as payer or payee. On A, go to Edit Profile and delete the profile. Expected: deletion is refused with a message naming the active subscription, and C plus the private payment links are still intact. Then tap Retry. Expected: it does not loop on the same generic failure.
Findings
- [MEDIUM] An active subscription with a saved contact now blocks profile deletion permanently, with no explanation — inline at
app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1116.
Reviewer model: claude-opus-5-5
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the existing active-subscription failure reproduces on device.
Findings:
- MEDIUM — Active subscriptions leave deletion in an unexplained Retry loop (
app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1123). I confirmed the existing finding with two authenticated native wallets, in both payer and payee roles. Bulk removal skips the subscribed peer, removes and blocks the unrelated contact, then this comparison throws the generic error. The dialog never explains the subscription; Retry repeats the failure. Check active subscriptions before destructive cleanup and show the actionable reason. Test 6 supplies the new evidence; I am keeping it in the existing thread.
I found no additional defect in the cleanup patch or its affected parent integration. The acknowledged publication-pause finding remains open; the author's reply tracks recovery separately.
QA for f72e0c0: Android 16 / API 36, Core 0.8.0-rc11 and Paykit 0.1.1-rc2. The unmodified head was built separately. Device cases used this head plus the fixture below: disposable authenticated identities, unfunded regtest wallets, the real Edit Profile screen, ViewModel, repositories and native SDK, with a homeserver fault proxy. Both installed APK hashes matched the built fixture APK.
Test 1: Manual 1 / J1 — private cleanup fails ✅ Failure stops before contact or profile deletion.
The proxy returned 503 for the native shared-state GET during private cleanup. After Cancel, the refreshed local inspector retained the session, both contacts and overrides, both sharing flags and no success feedback. Call counters and the proxy trace confirmed that contact/profile deletion was not entered.
1a.2x.mp4
Private cleanup failure and retained state, at 2× speed.
Test 2: Manual 1 / J1 — unconfirmed contact removal ✅ Unconfirmed removal keeps local evidence and stops profile deletion.
The trace showed the native write-ahead PUT accepted with 201, followed by shared-state publication rejected with 503. The failure dialog appeared. The refreshed inspector retained both contacts and overrides, session and sharing flags, with no profile deletion or success feedback.
1b.2x.mp4
Unconfirmed cleanup and refreshed state after Cancel, at 2× speed.
Test 3: Manual 1 / J1 — partial native removal ✅ Only the acknowledged contact and override disappear.
A native published marker retained one contact while the SDK acknowledged the other removal. Failure appeared before profile deletion. The refreshed inspector showed one contact and override; the persisted override key matched the retained marker. Public-contact publication was enabled only to stage this fixture variant.
1c.2x.mp4
Partial removal and retained evidence, at 2× speed.
Test 4: Manual 2 — profile deletion returns 404 ✅ Failure retains the session and sharing settings.
The trace confirmed profile GET 200 followed by DELETE 404. The app showed failure with no success feedback. The refreshed inspector retained its session and both sharing flags; previously acknowledged contact removals stayed applied.
2.2x.mp4
Rejected profile deletion and retained session, at 2× speed.
Test 5: J1 — Cancel and cold restart ✅ Normal startup restores the same identity without replaying profile deletion.
After unconfirmed contact cleanup and Cancel, a cold launch through the normal app entry restored the same identity on Home and Profile. The loaded profile and QR identifier matched the original owner. A profile GET returned 200, and no profile DELETE appeared during the 90-second proxy observation.
restart.2x.mp4
Normal cold startup and restored profile, at 2× speed.
Test 6: Active subscription — delete and Retry ❌ Both payer and payee get generic failure after unrelated-contact removal.
Two linked native wallets proposed and accepted a recurring request whose first period starts the following day; no payment or funds were needed. Each wallet had its subscribed peer plus an unrelated saved contact C. I ran Delete Profile and then Retry on both wallets.
Both runs removed C and its override, blocked C, and retained the active subscription and its peer. The dialog offered Retry/Disconnect without explaining the subscription. Retry produced the same failure. The inspectors showed two cleanup attempts and no profile deletion, success toast or success navigation. Native request records confirmed the payer/payee roles; persisted override keys confirmed which contact survived. This adds device evidence to the existing finding.
| Payee | Payer |
|---|---|
subscription-payee.2x.mp4 |
subscription-payer.2x.mp4 |
Initial deletion, retained subscription and repeated failure on Retry, at 2× speed. Private payment-link usability was not separately measured in this unfunded fixture.
Check 1: unmodified-head build and focused regressions ✅ The app build and affected component suites pass.
assembleDevDebug and focused testDevDebugUnitTest runs covering PubkyRepoTest, EditProfileViewModelTest, PrivatePaykitRepoTest and PaykitSdkServiceTest passed. XML reports contain no failures, errors or skips. These include contact-read errors, uncertain/partial removals, 404-like profile errors, private cleanup failure, local-finalization failure and active-subscription retention. git diff --check passed.
Check 2: delta and twin comparison ✅ The cleanup feature patch survives the additive parent merge.
2daec90 is an ancestor of f72e0c0. The six-file cleanup feature patch is preserved; the merged Paykit and restoration dependencies were reviewed with the reached cleanup paths. The native dependency upgrade from rc1 to rc2 and changed startup composition prompted fresh device runs instead of carrying the earlier revision-bound report.
Delta review of 2daec90..f72e0c0, the cleanup feature and its changed dependencies, plus open findings. Unrelated changes imported from the parent were not independently re-reviewed in full. Contact-read and local-finalization storage failures had component coverage rather than separate device cases. Original-owner/manual-retry reconciliation and native cross-device conditional removal remain outside this PR's stated scope.
Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup matches the scoped cleanup/error propagation at c26d7b13. This run adds no iOS device result.
Test fixture used by Tests 1–6
Not pushed. Provisions disposable native identities and contacts, establishes a native recurring request between two wallets, renders the actual Edit Profile screen, and exposes call/local-state evidence. Native calls return their real results. The public-contact policy is enabled to stage Test 3's retained marker. The fault proxy supplies Tests 1, 2 and 4; Test 5 launches the normal app entry. Fixture APK SHA-256: 915d1997fdb45dc11c8f69b7c4ccf6291a85d0b66c71be5f1f14df9044bd9a20.
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..7b18a44e0 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
android:usesCleartextTraffic="true"
tools:ignore="MissingApplicationIcon">
+ <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- test hook -->
+
<provider
android:name="to.bitkit.dev.DevToolsProvider"
android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 87c452f64..011d57041 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
suspend fun fetchPubkyFollows(publicKey: String): List<String> =
publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
+ internal suspend fun reviewPublishContact(key: String) { // test hook: provision real native retained marker
+ isSetup.await() // test hook
+ operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } } // test hook
+ } // test hook
+
suspend fun contactRecords(): List<ContactRecord> {
isSetup.await()
return operationLock.withLock {
@@ -1952,7 +1957,7 @@ internal object BitkitPaykitSdkConfig {
}
internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
- publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+ publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: native public-marker fixture
)
internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index e9105a40c..560146420 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,6 +208,7 @@ class PubkyService @Inject constructor(
}
suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
paykitSdkService.deletePaykitProfile()
}
@@ -216,6 +217,7 @@ class PubkyService @Inject constructor(
}
suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.reads++; to.bitkit.dev.CleanupProbe.note("contact read entered") // test hook
paykitSdkService.contactRecords()
}
@@ -252,6 +254,7 @@ class PubkyService @Inject constructor(
}
suspend fun removeContacts(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("contact remove entered count=${publicKeys.size}") // test hook
paykitSdkService.removeContacts(publicKeys)
}
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,175 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+ var reads = 0
+ var removals = 0
+ var deletes = 0
+ fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+ @Inject lateinit var service: PubkyService
+ @Inject lateinit var sdk: PaykitSdkService
+ @Inject lateinit var repo: PubkyRepo
+ @Inject lateinit var privateRepo: PrivatePaykitRepo
+ @Inject lateinit var pubkyStore: PubkyStore
+ @Inject lateinit var settings: SettingsStore
+ @Inject lateinit var keychain: Keychain
+ @Inject lateinit var wallet: WalletRepo
+ private var status by mutableStateOf("Preparing disposable native identity")
+ private var evidence by mutableStateOf("")
+ private var vm by mutableStateOf<EditProfileViewModel?>(null)
+ private var exited by mutableStateOf(false)
+ private var successToasts = 0 // test hook
+
+ override fun onNewIntent(intent: Intent) {
+ super.onNewIntent(intent)
+ lifecycleScope.launch {
+ try {
+ val peer = intent.getStringExtra("peer").orEmpty()
+ when (intent.getStringExtra("action")) {
+ "link" -> {
+ service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+ sdk.ensureLinkWithPeer(peer, 8u)
+ sdk.processOutboundPrivateMessages(peer)
+ sdk.receivePrivateMessages(peer)
+ repo.loadContacts()
+ pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+ }
+ "pump" -> {
+ sdk.ensureLinkWithPeer(peer, 8u)
+ sdk.receivePrivateMessages(peer)
+ sdk.processOutboundPrivateMessages(peer)
+ }
+ "propose" -> {
+ val now = java.time.Instant.now()
+ val start = now.plusSeconds(86400).toString()
+ val proposal = PaykitPaymentRequestProposalTerms(
+ amountValue = "0.00005", amountAsset = "btc",
+ paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+ proposalExpiresAt = now.plusSeconds(259200).toString(),
+ acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+ metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+ recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start)
+ )
+ val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+ CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+ sdk.processOutboundPrivateMessages(peer)
+ }
+ "accept" -> {
+ sdk.receivePrivateMessages(peer)
+ val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+ val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+ sdk.processOutboundPrivateMessages(peer)
+ CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+ }
+ "inspect" -> Unit
+ }
+ status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+ inspect()
+ } catch (e: Exception) {
+ status = "Native fixture action failed: ${e.message}"
+ CleanupProbe.note(status)
+ Log.e("CleanupProbe", "command", e)
+ }
+ }
+ }
+
+ override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+ setContent {
+ AppThemeSurface {
+ Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+ Text(status, Modifier.padding(horizontal = 12.dp))
+ Text(evidence, Modifier.padding(horizontal = 12.dp))
+ Row {
+ TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+ TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+ }
+ if (exited) Text("Deletion success navigation")
+ vm?.let { current ->
+ Box(Modifier.weight(1f)) {
+ EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+ }
+ }
+ }
+ }
+ }
+ lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+ lifecycleScope.launch {
+ try {
+ if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+ service.initialize()
+ if (intent.getBooleanExtra("resume", false)) {
+ repo.initialize().getOrThrow()
+ status = "Restored through normal profile initialization"
+ } else {
+ val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ val home = service.publicKeyFromSecret("00".repeat(32))
+ service.signUp(secret, home, null)
+ service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+ repo.initialize().getOrThrow()
+ val peers = (if (intent.getBooleanExtra("subscription", false)) listOf("22") else listOf("11", "22")).map { service.publicKeyFromSecret(it.repeat(32)) }
+ for ((index, peer) in peers.withIndex()) {
+ service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+ }
+ if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+ repo.loadContacts()
+ pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+ settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+ status = "Authenticated disposable identity"
+ }
+ CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0
+ vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+ CleanupProbe.note("READY owner=${repo.publicKey.value}")
+ inspect()
+ } catch (e: Exception) {
+ status = "Provisioning failed: ${e.message}"
+ CleanupProbe.note(status)
+ Log.e("CleanupProbe", "setup", e)
+ }
+ }
+ }
+
+ private suspend fun inspect() {
+ val state = settings.data.first()
+ val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+ val overrides = pubkyStore.data.first().contactProfileOverrides.size
+ evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts"
+ if (intent.getBooleanExtra("subscription", false)) {
+ val requests = sdk.paymentRequests()
+ val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+ val peers = sdk.linkedPeers()
+ evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+ CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { it.localRole to it.counterparty }}")
+ }
+ CleanupProbe.note(evidence)
+ }
+}Decisive inspector lines after deletion/Retry (native role records and persisted keys were checked as described above):
Private cleanup failure: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0
Unconfirmed contact removal: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Partial native removal: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
DELETE 404: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0
PAYEE after Retry: Session true; contacts 1; overrides 1; sharing true/true; reads 2, removals 2, deletes 0; success false; success toasts 0; active subscriptions 1; linked 1; blocked 1
PAYER after Retry: Session true; contacts 1; overrides 1; sharing true/true; reads 2, removals 2, deletes 0; success false; success toasts 0; active subscriptions 1; linked 1; blocked 1
Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: I found no new defects. The active-subscription finding is fixed at 7afdd3e: both native roles refuse before cleanup, and fresh confirmations succeed after cancellation. The acknowledged publication-pause issue remains tracked in the separate recovery follow-up.
QA for 7afdd3e: Android 16 / API 36, bitkit-core 0.8.0-rc11 and Paykit 0.1.1-rc2. The unmodified head was built separately. Device cases used this head plus the fixture below, with disposable authenticated identities, unfunded regtest wallets and a controlled homeserver proxy. Both installed APK hashes matched the built fixture APK.
Test 1: Manual 3 / J1 — active payee subscription ✅ The subscription instruction appears before any cleanup.
A future-dated native recurring request was accepted between two authenticated wallets. The payee saw “End active subscriptions before removing this profile.” No generic Retry dialog appeared. The refreshed inspector retained both contacts and overrides, the subscription, session and sharing flags. Native role records and call counters confirmed zero private pause, withdrawal, contact removal or profile deletion.
Recorded flow and state evidence, at 2× speed.
Test 2: Manual 3 / J1 — active payer subscription ✅ The payer receives the same instruction and keeps both contacts.
The counterpart wallet held the native payer record and an unrelated saved contact. Confirming Delete Profile emitted the localized instruction. The refreshed inspector and native records retained the active obligation, both contacts and overrides, session and sharing flags; all cleanup counters stayed zero. No generic Retry dialog appeared.
Recorded flow and state evidence, at 2× speed.
Test 3: Manual 3 / J1 — cancellation permits payee removal ✅ A new confirmation succeeds after the subscription ends.
The native SDK cancelled the disposable subscription, and its cancellation was delivered to the counterpart. After confirming that no active subscription remained, I explicitly confirmed removal again. The inspector showed one contact batch and one profile deletion, no contacts or overrides, cleared session/sharing flags, and the success toast/effect. The fixture displays the delivered navigation effect as a marker.
Recorded flow and state evidence, at 2× speed.
Test 4: Manual 3 / J1 — cancellation permits payer removal ✅ A new payer confirmation also completes removal.
After the native cancellation reached this wallet, its active-subscription count was zero. A fresh confirmation acknowledged contact and profile removal. The refreshed inspector showed the session and sharing flags cleared, no contacts or overrides, one contact batch, one profile deletion and the success toast/effect.
Recorded flow and state evidence, at 2× speed.
Test 5: Subscription preflight cannot read storage ✅ An unavailable subscription read stops before private cleanup.
An identity-scoped homeserver rule returned 503 for the shared-state GET. The existing failure dialog appeared. After Cancel, both contacts and overrides, session and sharing flags remained. The proxy trace confirmed the rejected read; the inspector recorded one preflight and zero private pauses, withdrawals, contact removals or profile deletions.
Recorded flow and state evidence, at 2× speed.
Test 6: Manual 1 / J1 — unconfirmed contact removal ✅ Unconfirmed publication retains local evidence and stops profile deletion.
The proxy accepted the native write-ahead PUT with 201, then rejected shared-state publication with 503. The failure dialog appeared. After Cancel, the refreshed inspector retained both contacts and overrides, session and sharing flags, with one contact-removal attempt and no profile deletion or success feedback.
Recorded flow and state evidence, at 2× speed.
Test 7: Manual 1 / J1 — partial native removal ✅ The retained-marker fixture keeps one contact and override.
A native published marker retained one contact while the SDK acknowledged the other removal. The profile-only batch reached the native SDK; the app then showed failure. After Cancel, the inspector showed one remaining contact and override, retained session/sharing flags and zero profile deletions or success feedback. Public-contact publication was enabled only to stage this fixture variant.
Recorded flow and state evidence, at 2× speed.
Test 8: Manual 2 — profile deletion returns 404 ✅ Failure retains the session and sharing settings.
The proxy trace confirmed profile GET 200 followed by DELETE 404. The app showed failure with no success feedback. After Cancel, the refreshed inspector retained the session and both sharing flags. Earlier acknowledged contact removals remained applied.
Recorded flow and state evidence, at 2× speed.
Check 1: exact-head build and component regressions ✅ The app build and affected suites pass.
assembleDevDebug and focused testDevDebugUnitTest runs covering PubkyRepoTest, PaykitSdkServiceTest and EditProfileViewModelTest passed on the unmodified head. XML reports contain no failures, errors or skips. These cover conservative subscription expiry, payer/payee records, unavailable reads, service-error wrapping, cancellation, incomplete contact cleanup and failed local finalization. git diff --check passed.
Check 2: earlier evidence and delta scope ✅ The unchanged private-withdrawal and cold-restart cases retain their earlier evidence.
I rely on the prior authenticated report for private-withdrawal failure and Cancel plus normal cold startup without deletion replay. Their cleanup/restart implementations and native dependency pins are unchanged. Contact publication, partial removal and profile-404 cases were driven again through the new profile-only batch.
Delta review of f72e0c0..7afdd3e, all 25 changed files, reached cleanup/cancellation paths and prior feedback. Unknown/malformed expiry, service wrappers, contact-read and local-finalization failures have component coverage rather than separate device fault cases. Manual-retry ownership and native cross-device conditional removal remain outside this PR’s stated scope.
Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at a2b2186 matches early subscription refusal, localized guidance and strict contact cleanup; iOS also checks at final SDK profile deletion. This run adds no iOS device result.
Native test fixture
Not pushed. Provisions disposable native identities and contacts, links two wallets, proposes/accepts/cancels a future-dated recurring request, renders the real Edit Profile screen with the normal toast components, and displays native/local state plus call counters. Deletion decisions and SDK results are unchanged. A native public marker stages partial retention; the proxy supplies the storage and DELETE faults. The success-navigation marker records the real ViewModel effect. Fixture APK SHA-256: f0e4191613c2ca51003932c9c4f8972cb04cd03f41443bf501fc646262f24d13.
diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..7b18a44e0 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
android:usesCleartextTraffic="true"
tools:ignore="MissingApplicationIcon">
+ <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- test hook -->
+
<provider
android:name="to.bitkit.dev.DevToolsProvider"
android:authorities="${applicationId}.devtools"
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..63ab9e245
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,186 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+ var reads = 0
+ var removals = 0
+ var withdrawals = 0
+ var pauses = 0
+ var preflights = 0
+ var deletes = 0
+ fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+ @Inject lateinit var service: PubkyService
+ @Inject lateinit var sdk: PaykitSdkService
+ @Inject lateinit var repo: PubkyRepo
+ @Inject lateinit var privateRepo: PrivatePaykitRepo
+ @Inject lateinit var pubkyStore: PubkyStore
+ @Inject lateinit var settings: SettingsStore
+ @Inject lateinit var keychain: Keychain
+ @Inject lateinit var wallet: WalletRepo
+ private var status by mutableStateOf("Preparing disposable native identity")
+ private var evidence by mutableStateOf("")
+ private var vm by mutableStateOf<EditProfileViewModel?>(null)
+ private var exited by mutableStateOf(false)
+ private var successToasts = 0 // test hook
+ private val toastQueue by lazy { to.bitkit.ui.shared.toast.ToastQueueManager(lifecycleScope) } // test hook: same app toast host
+
+ override fun onNewIntent(intent: Intent) {
+ super.onNewIntent(intent)
+ lifecycleScope.launch {
+ try {
+ val peer = intent.getStringExtra("peer").orEmpty()
+ when (intent.getStringExtra("action")) {
+ "link" -> {
+ service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+ sdk.ensureLinkWithPeer(peer, 8u)
+ sdk.processOutboundPrivateMessages(peer)
+ sdk.receivePrivateMessages(peer)
+ repo.loadContacts()
+ pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+ }
+ "pump" -> {
+ sdk.ensureLinkWithPeer(peer, 8u)
+ sdk.receivePrivateMessages(peer)
+ sdk.processOutboundPrivateMessages(peer)
+ }
+ "propose" -> {
+ val now = java.time.Instant.now()
+ val start = now.plusSeconds(86400).toString()
+ val proposal = PaykitPaymentRequestProposalTerms(
+ amountValue = "0.00005", amountAsset = "btc",
+ paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+ proposalExpiresAt = now.plusSeconds(259200).toString(),
+ acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+ metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+ recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start)
+ )
+ val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+ CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+ sdk.processOutboundPrivateMessages(peer)
+ }
+ "accept" -> {
+ sdk.receivePrivateMessages(peer)
+ val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+ val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+ sdk.processOutboundPrivateMessages(peer)
+ CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+ }
+ "cancel" -> {
+ val record = sdk.paymentRequests().first { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+ sdk.cancelPaymentRequest(record.counterparty, record.paymentRequestId, "Disposable cleanup test ended")
+ sdk.processOutboundPrivateMessages(peer)
+ }
+ "inspect" -> Unit
+ }
+ status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+ inspect()
+ } catch (e: Exception) {
+ status = "Native fixture action failed: ${e.message}"
+ CleanupProbe.note(status)
+ Log.e("CleanupProbe", "command", e)
+ }
+ }
+ }
+
+ override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+ setContent {
+ AppThemeSurface {
+ val currentToast by toastQueue.currentToast.collectAsState() // test hook
+ Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+ Text(status, Modifier.padding(horizontal = 12.dp))
+ Text(evidence, Modifier.padding(horizontal = 12.dp))
+ Row {
+ TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+ TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+ }
+ if (exited) Text("Deletion success navigation")
+ vm?.let { current ->
+ Box(Modifier.weight(1f)) {
+ EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+ }
+ }
+ }
+ to.bitkit.ui.components.ToastOverlay(toast = currentToast, onDismiss = toastQueue::dismissCurrentToast) // test hook
+ }
+ }
+ lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> toastQueue.enqueue(toast); if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+ lifecycleScope.launch {
+ try {
+ if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+ service.initialize()
+ if (intent.getBooleanExtra("resume", false)) {
+ repo.initialize().getOrThrow()
+ status = "Restored through normal profile initialization"
+ } else {
+ val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ val home = service.publicKeyFromSecret("00".repeat(32))
+ service.signUp(secret, home, null)
+ service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+ repo.initialize().getOrThrow()
+ val peers = (if (intent.getBooleanExtra("subscription", false)) listOf("22") else listOf("11", "22")).map { service.publicKeyFromSecret(it.repeat(32)) }
+ for ((index, peer) in peers.withIndex()) {
+ service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+ }
+ if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+ repo.loadContacts()
+ pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+ settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+ status = "Authenticated disposable identity"
+ }
+ CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0; CleanupProbe.withdrawals = 0; CleanupProbe.pauses = 0; CleanupProbe.preflights = 0
+ vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+ CleanupProbe.note("READY owner=${repo.publicKey.value}")
+ inspect()
+ } catch (e: Exception) {
+ status = "Provisioning failed: ${e.message}"
+ CleanupProbe.note(status)
+ Log.e("CleanupProbe", "setup", e)
+ }
+ }
+ }
+
+ private suspend fun inspect() {
+ val state = settings.data.first()
+ val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+ val overrides = pubkyStore.data.first().contactProfileOverrides.size
+ evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts; preflights ${CleanupProbe.preflights}, pauses ${CleanupProbe.pauses}, withdrawals ${CleanupProbe.withdrawals}"
+ if (session && intent.getBooleanExtra("subscription", false)) {
+ val requests = sdk.paymentRequests()
+ val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+ val peers = sdk.linkedPeers()
+ evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+ CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { it.localRole to it.counterparty }}")
+ }
+ CleanupProbe.note(evidence)
+ }
+}
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index ec6558f32..e99799126 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -361,6 +361,7 @@ class PrivatePaykitRepo @Inject constructor(
}
suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
+ to.bitkit.dev.CleanupProbe.pauses++ // test hook
_isProfileDeletionInProgress.update { true }
invalidateContactPreparation()
}
@@ -506,6 +507,7 @@ class PrivatePaykitRepo @Inject constructor(
deferAppSync: Boolean,
): Result<Unit> = withContext(serializedDispatcher) {
runSuspendCatching {
+ to.bitkit.dev.CleanupProbe.withdrawals++; to.bitkit.dev.CleanupProbe.note("private withdrawal entered") // test hook
updateContactSharingCleanupPending(true)
removePublishedEndpoints().getOrThrow()
if (!deferAppSync) {
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 076e3fa65..00968fc62 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
suspend fun fetchPubkyFollows(publicKey: String): List<String> =
publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
+ internal suspend fun reviewPublishContact(key: String) { // test hook: stage a real native retained marker
+ isSetup.await()
+ operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } }
+ }
+
suspend fun contactRecords(): List<ContactRecord> {
isSetup.await()
return operationLock.withLock {
@@ -1977,7 +1982,7 @@ internal object BitkitPaykitSdkConfig {
}
internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
- publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+ publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: retained-marker fixture
)
internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index 18cf81711..04610523d 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,14 +208,17 @@ class PubkyService @Inject constructor(
}
suspend fun checkProfileDeletionAllowed() = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.preflights++; to.bitkit.dev.CleanupProbe.note("profile preflight entered") // test hook
paykitSdkService.checkProfileDeletionAllowed()
}
suspend fun removeContactsForProfile(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("profile contact batch entered") // test hook
paykitSdkService.removeContactsForProfile(publicKeys)
}
suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
paykitSdkService.deletePaykitProfile()
}
@@ -224,6 +227,7 @@ class PubkyService @Inject constructor(
}
suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+ to.bitkit.dev.CleanupProbe.reads++ // test hook
paykitSdkService.contactRecords()
}
Decisive refreshed inspector lines (native roles and proxy responses checked as described above):
3-payee: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0; active subscriptions 1; linked 1; blocked 0
3-payer: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0; active subscriptions 1; linked 1; blocked 0
3-ended-payee: Session false; contacts 0; overrides 0; sharing false/false; reads 1, removals 1, deletes 1; success true; success toasts 1; preflights 3, pauses 1, withdrawals 1
3-ended-payer: Session false; contacts 0; overrides 0; sharing false/false; reads 1, removals 1, deletes 1; success true; success toasts 1; preflights 3, pauses 1, withdrawals 1
subscription-read: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0
1-unknown: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1
1-partial: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1
2-404: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full reassessment of the whole PR at 7afdd3e, compared with its merge base 5fe0bf6. That merge base is the head of dependency #1688. A full reassessment was needed because the base changed since the earlier assessment (99b446a → 5fe0bf6) and the target-branch changes touch PubkyRepo.kt and PubkyRepoTest.kt. No coverage was carried over.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The stricter cleanup handling works as intended. A failed private-endpoint withdrawal, an unreadable contact list, a partial or unknown contact removal, or a profile-deletion error now stops deletion before sign-out. Only contacts confirmed as removed have their local data and overrides cleared. If clearing local state fails after deletion, no success effect is emitted. Because Bitkit sets PrivateOnly contact sharing by default, the SDK should not keep contacts behind public markers. One gap remains in the new active-subscription preflight: it blocks deletion on fixed-term subscriptions, but Bitkit gives users no way to cancel those (see finding). iOS twin #1124 at a2b2186 has the same mechanism: its expiry predicate is the same, and SubscriptionsView also hides Cancel when endsAt is set.
Validation: source analysis at the pinned revisions, including the SDK contract in paykit-rs v0.1.1-rc2 (01de5dd), which is the version pinned in the catalog. That covers contact_records, remove_contacts_and_block_peers skip rules, payment_requests and cancel_payment_request. No Android SDK was available locally, so I did not run any tests. At this head, hosted CI and lint passed. The new repository, service and view-model tests were inspected but not run by me. Journey J1 and manual tests 1–3 need the authenticated fault/subscription fixture and have not been run.
Device testing: not performed in this review.
Suggested additional test cases
- Android: accept an incoming recurring subscription whose terms include a future
ends_at(from an external issuer, disposable identities). Open Subscriptions and confirm no Cancel action is offered. Then try Edit Profile → Delete (or Unlink for a Ring identity). Expected: the user gets a way forward (for example a Disconnect option or guidance that matches the subscription's end date), not an instruction they cannot follow. - Android: make
closeAndClearfail after a successful remote deletion. Expected: the error toast appears, the screen stays open, and a second Delete tap leads to the failure dialog, where Disconnect clears local state and leaves the profile screens.
Earlier findings reconciled at this revision: 1 still present, 1 verified fixed.
Findings
- [MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them — inline at
app/src/main/java/to/bitkit/services/PaykitSdkService.kt:772.
Existing concerns
- [LOW] A stopped profile deletion leaves private payment endpoints withdrawn until the next foreground or network event — still unresolved; already tracked.
Reviewer model: claude-opus-5-5
Reasoning effort: high
| operationLock.withLock { | ||
| withStateRevisionTracking { handle -> | ||
| val now = nowMillis() | ||
| if (completeSdkCall { handle.paymentRequests() }.any { it.hasActiveSubscription(now) }) { |
There was a problem hiding this comment.
[MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them
Trigger: the user has an accepted recurring subscription (as payer or payee) whose terms carry a future ends_at. Bitkit accepts incoming proposals like this: PaykitPaymentRequestRepo.accept has no endsAt guard, and SubscriptionsScreen shows them with an expiry date. The user then confirms Delete Profile, or Unlink for a Ring identity.
Mechanism: checkProfileDeletionAllowed throws ActiveSubscription for any ACTIVE_RECURRING record where endsAt == null || endsAt > now. An unparseable endsAt also counts as active. EditProfileViewModel.attemptDeleteProfile (line 277) and PubkyRepo.deleteProfile (line 1108) both call it. On that error the view model shows only the profile__delete_active_subscription toast ("End active subscriptions before removing this profile.") and does not open the failure dialog. That dialog is the only place disconnectProfile can be reached (EditProfileScreen.kt:199-206). Meanwhile, the unchanged PaykitSubscription.canCancel (PaykitSubscription.kt:223) returns false whenever recurrence.endsAt != null. PaykitPaymentRequestRepo.cancel (line 1335) enforces this, and SubscriptionsScreen.kt:652 hides the Cancel button.
Consequence: this path would leave the user unable to delete or unlink the profile, and without the Disconnect fallback, until the term expires or the counterparty cancels. If ends_at cannot be parsed, removal is blocked permanently. The instruction in the toast cannot be followed in the app. No funds or data are lost.
Expected: the preflight and the cancellation policy should agree. Either let fixed-term subscriptions be ended in the app, or for subscriptions the user cannot end, show their end date and still offer a recovery path (for example the Disconnect option). Manual case 3 itself expects the user to "End the subscription and confirm again." The new test profile removal conservatively blocks unknown or future subscription expiry confirms the blocking side, but no test covers whether such a subscription can be ended.
Evidence basis: source analysis at 7afdd3e and paykit-rs v0.1.1-rc2. I did not execute any tests. iOS twin #1124 (a2b2186) has the same mechanism in PaykitSdkService.requireNoActiveSubscriptions together with SubscriptionsView.swift:509.
There was a problem hiding this comment.
The parent guidance repair is published in 98797f2, composed with current #1688 in 815e85c. It preserves the fixed-term cancellation policy and conservative removal guard. The typed profile refusal shows the latest end time only when every blocking obligation has a valid finite expiry; unreadable/missing expiry stays unknown and blocking. Final combined-tree validation passed 4,721 units plus compile/format/Detekt/Android Lint with zero findings; all seven live-base commits are verified. The fresh original-owner local-only Disconnect option is still being composed and validated in child #1693, so I am keeping this thread open until that complete path and its applicable acceptance are verified. No authenticated fixed-term journey pass is claimed yet.
There was a problem hiding this comment.
Checked at 815e85c with authenticated native payer/payee wallets: two finite obligations show the latest expiry before any cleanup, and adding an open-ended obligation removes the date. Fresh confirmations succeed after open-ended cancellation and natural finite expiry. The guidance part holds, but this head still has no independent local Disconnect; the recovery described in child #1693 is not part of this head. Leaving this thread open.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 815e85c after the comparison base changed from 5fe0bf6c to 2aac53ed. No prior coverage was inherited.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
Profile deletion stops before sign-out when the subscription read, private endpoint withdrawal, contact read, or contact acknowledgement is incomplete. An active payer or payee recurring subscription, including one with no saved contact, is refused before that cleanup, and the toast shows the latest finite expiry only when every blocking record has a readable end. Ordinary bulk contact removal still skips subscribed keys and continues with the others. A 404-like profile deletion error stays a failure and does not sign out. That matches bitkit-android#1688 at 2aac53ed, which removed session-retry deletion: this head calls deleteProfile and does not sign in again after an uncertain response.
Compared with bitkit-ios#1124 at a2b21861, stopping on incomplete cleanup and blocking on active subscriptions match. That iOS profile toast still says to end subscriptions and does not show an expiry; this Android head shows “Active subscriptions prevent removing this profile.” plus the latest finite expiry, which is the fixed-term behavior in this PR. The journey files differ only in that refusal action.
Device testing: not performed in this review. J1 and manual cases 1–3 still need the authenticated fault proxy and disposable subscription fixture named in the PR. PubkyRepoTest.kt, PaykitSdkServiceTest.kt, and EditProfileViewModelTest.kt were inspected and not executed here. Hosted build and lint had passed on this head; e2e shards were still pending.
Non-blocking consistency suggestions
- At iOS
a2b21861, profile refusal uses “End active subscriptions before removing this profile.” and omits the expiry. Android shows the prevent-removal sentence and the latest finite expiry. Align the twin profile toast andprofile-cleanup-failure.xmlaction when that fixed-term guidance lands.
Earlier findings reconciled at this revision: 2 still present.
Findings
- [MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them — inline at
app/src/main/java/to/bitkit/services/PaykitSdkService.kt:796. Still present from the earlier review.
Existing concerns
- [LOW] A stopped profile deletion leaves private payment endpoints withdrawn until the next foreground or network event — still unresolved; already tracked.
Reviewer model: grok-4.7
Reasoning effort: high
| if (subscriptions.isEmpty()) return | ||
| val endDates = subscriptions.mapNotNull { it.subscriptionEndsAtMillis() } | ||
| val latestEnd = if (endDates.size == subscriptions.size) endDates.maxOrNull() else null | ||
| throw PubkyContactError.ActiveProfileSubscriptions(latestEnd) |
There was a problem hiding this comment.
[MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them
Trigger: the user has an accepted recurring subscription (as payer or payee) whose terms carry a future ends_at. Bitkit accepts incoming proposals like this: PaykitPaymentRequestRepo.accept has no endsAt guard, and SubscriptionsScreen shows them with an expiry date. The user then confirms Delete Profile, or Unlink for a Ring identity.
Mechanism: checkProfileDeletionAllowed throws ActiveSubscription for any ACTIVE_RECURRING record where endsAt == null || endsAt > now. An unparseable endsAt also counts as active. EditProfileViewModel.attemptDeleteProfile (line 277) and PubkyRepo.deleteProfile (line 1108) both call it. On that error the view model shows only the profile__delete_active_subscription toast ("End active subscriptions before removing this profile.") and does not open the failure dialog. That dialog is the only place disconnectProfile can be reached (EditProfileScreen.kt:199-206). Meanwhile, the unchanged PaykitSubscription.canCancel (PaykitSubscription.kt:223) returns false whenever recurrence.endsAt != null. PaykitPaymentRequestRepo.cancel (line 1335) enforces this, and SubscriptionsScreen.kt:652 hides the Cancel button.
Consequence: this path would leave the user unable to delete or unlink the profile, and without the Disconnect fallback, until the term expires or the counterparty cancels. If ends_at cannot be parsed, removal is blocked permanently. The instruction in the toast cannot be followed in the app. No funds or data are lost.
Expected: the preflight and the cancellation policy should agree. Either let fixed-term subscriptions be ended in the app, or for subscriptions the user cannot end, show their end date and still offer a recovery path (for example the Disconnect option). Manual case 3 itself expects the user to "End the subscription and confirm again." The new test profile removal conservatively blocks unknown or future subscription expiry confirms the blocking side, but no test covers whether such a subscription can be ended.
Evidence basis: source analysis at 7afdd3e and paykit-rs v0.1.1-rc2. I did not execute any tests. iOS twin #1124 (a2b2186) has the same mechanism in PaykitSdkService.requireNoActiveSubscriptions together with SubscriptionsView.swift:509.
Still present at 815e85c (first reported at 7afdd3e): At 815e85c a future finite endsAt still refuses Delete Profile and Unlink before cleanup, and Bitkit still offers no Cancel for that subscription. The toast now says active subscriptions prevent removal and, when every blocking record has a readable finite end, adds the latest expiry. An unparseable endsAt still blocks with no date. The failure dialog stays closed, so Disconnect is still unreachable on this path. The open thread #1690 (comment) records the author's plan to add that recovery in #1693; that follow-up is not in this head. No tests were executed.
There was a problem hiding this comment.
The local-only recovery path is now published in #1693: product commit 0dcad96, composed with this parent at 564a54d. Edit Profile offers a fresh explicit Disconnect from this device confirmation without first attempting deletion or entering the failure dialog. It binds the original owner/UUID, performs no remote cleanup, preserves strict credential-before-watermark cleanup, and leaves fixed-term cancellation policy unchanged. Final combined-tree validation passed 4,867 tests and required format/compile/lint; all nine live-base signatures verify locally and on GitHub. The dedicated authenticated fixed-term/refusal/Disconnect/restart journey is still unfinished, and new hosted CI is pending, so this thread remains open. The separate original-grant remote-retry finding is also not claimed fixed. Manual case 3 now consistently says active-subscription refusal; its existing fixed-term/open-ended distinctions are preserved.
There was a problem hiding this comment.
Confirmed the finite-expiry guidance at 815e85c on both native roles, with contacts/overrides/session/sharing retained and zero cleanup. This head still does not offer Disconnect on subscription refusal; the separately published local-only recovery is in child #1693. This review does not certify the child, so the finding remains open here.
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
There was a problem hiding this comment.
Verdict: 💬 Comment — expiry guidance is verified; the existing local-Disconnect finding remains open at this head.
Findings: I found no new defects in this delta. The finite-expiry guidance is fixed. The existing recovery finding still applies to this head: fixed-term cancellation remains unavailable, and this refusal does not expose Disconnect. The author tracks that separate recovery in bitkit-android#1693 fix: bind profile removal to its original owner. The acknowledged publication-pause concern also remains in its existing follow-up; I am not duplicating either finding.
QA for 815e85c: Android 16 / API 36, bitkit-core 0.8.0-rc11 and Paykit 0.1.1-rc2. I built the unmodified head separately. Device cases used this head plus an unpublished debug fixture with disposable authenticated identities, unfunded regtest wallets, the real Edit Profile screen/ViewModel/repositories/native SDK and controlled homeserver faults. Hooks provision data, expose state/call counters and suspend selected boundaries; they do not replace deletion decisions or native results. The screen exit effect is displayed as a marker. Installed APK hashes matched the fixture build.
Fixed-term payee sees the latest subscription expiry ✅ The payee sees the latest finite expiry before any cleanup.
Manual 3 / J1: two native recurring requests were accepted between disposable wallets, with different finite end times. The toast displayed the later expiry. The native terms and toast log independently confirm the exact minute. The inspector retained both contacts and overrides, session and sharing flags, with zero private pauses, withdrawals, contact removals or profile deletions. No generic Retry dialog appeared.
Recorded flow and state evidence, at 2× speed.
Fixed-term payer sees the latest subscription expiry ✅ The payer receives the same latest-expiry refusal.
Manual 3 / J1: the counterpart wallet held both accepted payer records and an unrelated contact. Its toast displayed the later finite expiry, checked against the native records and toast log. Both contacts and overrides, session and sharing flags remained; every cleanup counter stayed zero.
Recorded flow and state evidence, at 2× speed.
An open-ended subscription suppresses the expiry date ✅ An open-ended obligation removes the date from the refusal.
Manual 3 / J1: I added and accepted an open-ended native recurring request alongside the two finite requests. The refusal retained its neutral title and had no expiry description. Contacts, overrides, session and sharing flags remained, with no cleanup. Missing or malformed expiry is covered separately by the component checks below.
Recorded flow and state evidence, at 2× speed.
Payee removal succeeds after subscriptions end ✅ A fresh confirmation succeeds after the obligations end.
Manual 3 / J1: the disposable open-ended request was cancelled through the native SDK and delivered to the counterpart. The fixed-term requests were allowed to expire naturally; no fixed-term cancellation bypass was used. A fresh payee confirmation then acknowledged contact and profile removal, cleared session/sharing flags and emitted success. The fixture displays the real screen exit effect as a navigation marker.
Recorded flow and state evidence, at 2× speed.
Payer removal succeeds after subscriptions end ✅ The payer also completes removal after the obligations end.
Manual 3 / J1: after open-ended cancellation and natural finite expiry, a fresh payer confirmation acknowledged the contact batch and profile deletion. The inspector showed no contacts or overrides, cleared session/sharing flags and the success toast/exit effect.
Recorded flow and state evidence, at 2× speed.
Private withdrawal failure stops deletion ✅ Private withdrawal failure prevents contact and profile deletion.
Manual 1 / J1: a test-only suspension held entry to withdrawal after the real subscription preflight. The homeserver proxy then returned 503 for the native shared-state read. After Cancel, both contacts and overrides, session and sharing flags remained. The proxy trace and call counters confirm that contact/profile deletion never started.
Recorded flow and state evidence, at 2× speed.
Unconfirmed contact removal preserves local evidence ✅ Unconfirmed native removal keeps local evidence and stops deletion.
Manual 1 / J1: the native write-ahead PUT returned 201, then shared-state publication returned 503. The app showed the failure dialog. After Cancel, both local contacts and overrides, session and sharing flags remained, with one contact batch and no profile deletion or success feedback.
Recorded flow and state evidence, at 2× speed.
Partial contact removal keeps the retained contact ✅ Only the acknowledged contact and override disappear.
Manual 1 / J1: a real native public marker retained one contact while the SDK acknowledged removal of the other. The app reported failure before profile deletion. After Cancel, the inspector retained one contact and its override, session and sharing flags, without success feedback. Public-contact publication was enabled solely to stage this retained-marker variant.
Recorded flow and state evidence, at 2× speed.
A profile deletion error retains the session ✅ A profile-deletion error keeps the session and sharing settings.
Manual 2 / J1: the proxy confirmed profile GET 200 followed by DELETE 404. The app showed failure with no success feedback. After Cancel, session and both sharing settings remained; earlier acknowledged contact removals stayed applied.
Recorded flow and state evidence, at 2× speed.
Cancel and cold restart do not replay deletion ✅ Normal cold startup restores the original profile without replaying deletion.
J1: after unconfirmed contact cleanup and Cancel, I cold-launched the normal app entry. Home and Profile showed the original identity; the QR identifier matched the captured owner. A separate proxy observer, started before launch, saw successful profile reads and no profile DELETE for more than 90 seconds. The initial USDT introduction was declined to reach Home.
Recorded flow and state evidence, at 2× speed.
Failed local finalization does not report success ✅ A failed local write emits no deletion success and can recover through Disconnect.
Additional reviewer case: after acknowledged remote deletion, a test-only suspension allowed an actual obstruction at the private-cache write destination after staging a nonempty cache. The app showed Error, stayed on Edit Profile and emitted neither success nor the deletion Retry dialog. A new Delete confirmation opened the ordinary failure dialog. After removing the obstruction, Disconnect completed local cleanup and emitted the exit effect without a deletion-success toast.
Recorded flow and state evidence, at 2× speed.
Check: unmodified-head build and component regressions ✅ The build and affected component suites pass.
assembleDevDebug and focused testDevDebugUnitTest runs covering PubkyRepoTest, PaykitSdkServiceTest and EditProfileViewModelTest passed before adding hooks. Their XML reports contain no failures, errors or skips. Coverage includes malformed/missing expiry, aggregate expiry ordering, unavailable subscription/contact reads, typed errors through service wrappers, cancellation, incomplete contact cleanup and failed local finalization. These are component checks, with native/network resources mocked where the tests specify. git diff --check passed for the reviewed delta. Full lint and the unrelated E2E suite were not rerun in this review.
All planned device cases above were driven. I re-ran cleanup and cold-start cases because merged identity/background-admission code touched shared dependencies, rather than carrying the earlier authenticated report onto this head. Unreadable/malformed expiry and contact-read failures have component coverage rather than separate device faults. Safe manual Retry/Disconnect ownership and native cross-device conditional contact removal remain outside this PR’s stated scope.
Delta review of 7afdd3e..815e85c, with contiguous ancestry: the fixed-term guidance changes, their tests/translations, reached cleanup paths and affected parent composition. Unrelated send/UI functionality imported from the parent was not independently re-reviewed in full or driven by these cleanup cases.
Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at b14d5a1b matches strict cleanup, early subscription refusal and latest-known-expiry selection. Its toast formats a date; Android includes the time. The cleanup journey matches. This run adds no iOS device result.
Fixture APK SHA-256: 21520bef20b7bd2e3d58b7a6c72cca07695ab46e926c76395f3c911ad0762faa for the subscription, cleanup and restart cases. The local-finalization case uses the same product code with an added debug-only cache-staging hook; its APK SHA-256 is 0c18e1310a70fa2e5caf70bd8a980c122c17d16ec056a5ff5a0f0c92d60da8b9. All wallets and identities were disposable; no payment or live funds were needed.
Reviewed by gpt-6.1-sol-high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to 815e85c. No new findings.
7afdd3e3b and 98797f244 block profile removal while a recurring subscription is active (ACTIVE_RECURRING, with endsAt null, unparseable or in the future). This applies as payer and payee, with or without a contact. The toast shows the latest end date only when every blocker has a parsable end. Open-ended subscriptions can be ended in-app by either role, and wallet wipe never goes through this check, so nobody is locked out of wiping. The fixed-term case is piotr-iohk's open thread.
The rule matches synonymdev/bitkit-ios#1124 with two small differences. Android skips the check inside deletePaykitProfile and for an empty batch, where iOS checks both. Android's toast shows date and time, iOS only the date.
Device gate (partial): 815e85c, not run yet. Pending: profile-cleanup-failure.xml with two linked wallets (actions 1–3), and actions 4–11 with the homeserver fault proxy. l10n: profile__delete_active_subscription changed in every locale.
|
Device gate (partial):
Still running: actions 4–11, which need the homeserver fault proxy. |
|
Device gate (partial):
Actions 1–3 and l10n are in the previous partial. |
There was a problem hiding this comment.
Verdict: 💬 Comment — no new defects; the existing local-Disconnect finding remains open at this head.
Findings: I found no new defects in this delta. The existing recovery finding still applies: subscription refusal does not offer independent local Disconnect. The publication-pause concern also remains acknowledged. Their separate recovery is tracked in bitkit-android#1693 fix: bind profile removal to its original owner; I am not duplicating them or certifying that child.
QA for 7ef3432: Android 16 / API 36, bitkit-core 0.8.0-rc11 and Paykit 0.1.1-rc2. I built the unmodified head separately. Recorded cases used this head plus unpublished debug controls, disposable authenticated identities, unfunded regtest wallets and a controlled homeserver proxy. The real Edit Profile screen, ViewModel, repositories and native SDK made the deletion decisions. Hooks provision data, expose state/call counts and suspend selected boundaries; they do not replace SDK results. Installed APK hashes matched the fixture build.
Fixed-term payee sees the latest subscription expiry ✅ The payee sees the latest finite expiry before cleanup.
Manual 3 / J1: two native recurring requests were accepted between disposable wallets, with different finite end times. The payee toast showed the later expiry, checked against native terms and the toast log. Contacts, overrides, session and sharing settings remained, with zero cleanup calls and no generic Retry dialog.
Recorded flow and state evidence, at 2× speed.
Fixed-term payer sees the latest subscription expiry ✅ The payer receives the same latest-expiry refusal.
Manual 3 / J1: the counterpart held both accepted payer obligations and an unrelated contact. Its latest-expiry refusal retained both contacts and overrides, session and sharing flags. Cleanup counters stayed zero.
Recorded flow and state evidence, at 2× speed.
An open-ended subscription suppresses the expiry date ✅ An open-ended obligation removes the date from the refusal.
Manual 3 / J1: an accepted open-ended obligation alongside the finite terms suppressed the date. The toast log and native records confirm this; contacts, overrides, session and sharing settings remained without cleanup.
Recorded flow and state evidence, at 2× speed.
Payee removal succeeds after subscriptions end ✅ Fresh payee confirmation succeeds after the obligations end.
Manual 3 / J1: the disposable open-ended obligation was cancelled through the native SDK and delivered to the counterpart. Finite terms expired naturally. A fresh confirmation then acknowledged contact/profile removal, cleared session/sharing settings and emitted success. The fixture renders the real screen exit effect as a marker.
Recorded flow and state evidence, at 2× speed.
Payer removal succeeds after subscriptions end ✅ Fresh payer confirmation also completes removal.
Manual 3 / J1: after native open-ended cancellation and natural finite expiry, a fresh payer confirmation completed contact/profile removal. The inspector showed no contacts or overrides, cleared session/sharing flags and the success toast/exit effect.
Recorded flow and state evidence, at 2× speed.
Private withdrawal failure stops deletion ✅ Withdrawal failure prevents contact and profile deletion.
Manual 1 / J1: a debug suspension held withdrawal after the real preflight. The homeserver proxy then returned 503 for the native shared-state read. After Cancel, contacts, overrides, session and sharing flags remained; counters and the proxy trace show no contact/profile deletion.
Recorded flow and state evidence, at 2× speed.
Unconfirmed contact removal preserves local evidence ✅ Unconfirmed cleanup retains local evidence and stops deletion.
Manual 1 / J1: the native write-ahead PUT was accepted with 201, then shared-state publication returned 503. Failure appeared before profile deletion. After Cancel, local contacts and overrides, session and sharing settings remained without success feedback.
Recorded flow and state evidence, at 2× speed.
Partial contact removal keeps the retained contact ✅ Only acknowledged contact data is cleared.
Manual 1 / J1: a native public marker retained one contact while the SDK acknowledged the other removal. Only the acknowledged contact and override disappeared. Failure prevented profile deletion; session and sharing settings remained. Public-contact publication was enabled solely to stage this retained-marker variant.
Recorded flow and state evidence, at 2× speed.
A profile deletion error retains the session ✅ A deletion error keeps the session and sharing settings.
Manual 2 / J1: the proxy trace shows profile GET 200 followed by DELETE 404. Failure appeared without success feedback. After Cancel, session and sharing settings remained; earlier acknowledged contact removals stayed applied.
Recorded flow and state evidence, at 2× speed.
Cancel and cold restart do not replay deletion ✅ Cold startup restores the original profile without deletion replay.
J1: after unconfirmed contact cleanup and Cancel, I cold-launched the normal app entry. The original profile identity was checked in the normal UI and against its captured owner. Proxy observation covered 90 seconds, with successful profile reads and no profile DELETE.
Recorded flow and state evidence, at 2× speed.
Failed local finalization does not report success ✅ Failed local persistence emits no success and can recover through Disconnect.
Additional reviewer case: after acknowledged remote deletion, a debug suspension staged a nonempty private cache so finalization required a real write. Obstructing that destination produced Error without success or the deletion Retry dialog. A new Delete confirmation opened ordinary failure; after storage recovery, Disconnect completed local cleanup and emitted exit without a deletion-success toast.
Recorded flow and state evidence, at 2× speed.
Check: unmodified-head build and component regressions ✅ The build and affected suites pass.
assembleDevDebug and focused testDevDebugUnitTest runs passed. XML reports show no failures, errors or skips in the profile repository/ViewModel, SDK, private/public sharing, settings coordination, backup, reservation, wallet, allowance executor and manifest suites. They cover unreadable/malformed expiry, subscription/contact read errors, service wrapping, incomplete cleanup, cancellation, ownership and local finalization. The manifest check passed locally; that does not explain or waive its earlier hosted failure. git diff --check passed. Full lint and unrelated E2E were not rerun in this review.
All planned device cases above were driven. I repeated cleanup and restart cases because the merged sharing, reservation and startup dependencies changed. Unreadable/malformed expiry and contact-read failures retain component coverage rather than separate device faults. Safe destructive Retry/Disconnect ownership, Ring-specific recovery and native cross-device conditional removal remain outside this run’s device coverage.
The prior authenticated report supplies the previous-head comparison; the additional device report proves open-ended refusal in both roles and cancellation progress at 815e85c. The later partial report adds withdrawal-failure, Cancel and restart evidence at 815e85c. Those are revision-scoped reports, not additional current-head passes.
Delta review of 815e85c..7ef3432, with contiguous ancestry, changed production paths and open findings. The cleanup feature patch is identical against the new parent. Device coverage focuses on profile cleanup and subscription refusal; imported balance UI, send, backup/restore and address-publication journeys were not driven.
Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at de683f3 matches strict cleanup, early subscription refusal and latest-known-expiry selection. iOS also checks subscriptions at final SDK deletion; its toast shows the date, while Android includes the time. This run adds no iOS device result.
Reviewed by gpt-6.1-sol-high
Current parent composition — October 30, 22:49 UTC
Signed additive merge
7ef3432087568f16d34a983d90034ae4871af404incorporates #1688 at3001faf36a6e74e04e8cc1aac80c4bc8a7f4e714. The child patch is byte-identical. Final tree5e23c7580c26d1418c3ed7379ec5dfceab7f07a3passed required format, compile, 4,819 unit tests / 260 suites, and full Detekt/Android Lint with zero failures/errors/skips or findings. Production compilation and all lint analyses executed. Complete logs and structured reports were inspected; all 1,773 frozen regular inputs and six symlinks match. Known provider cursor/JVM warnings and historical Gradle deprecation records remain retained and attributed.The independent affected-composition review remains valid: its only subsequent input difference is the separately reviewed upstream test-diagnostic change. All eight live-parent range commits verify locally and on GitHub (
verified=true,reason=valid). This PR remains ready. Current-head CI passed 4,819 units, and lint has zero findings. E2E passed 39 cases on all seven first in-job attempts, with one explicit settings skip and no failed attempt. Synthetice5335ab2equals the published tree against exact parent3001faf3. Complete available job/runtime logs and all uploaded unit/lint reports were inspected; nine archives match their digests and ZIP CRCs. Hosted JUnit XML and Gradle problems HTML were not uploaded. Migration skipped; stock E2E does not satisfy the authored authenticated cleanup/recovery cases. The inherited manifest failure is not causally explained by the passing local run. Earlier fixed-term/device evidence stays revision-scoped; remaining safe-recovery and authenticated fault cases stay assigned to #1693/#1687 without being marked passed here.Twin: synonymdev/bitkit-ios#1124
Dependency: #1688
This PR stops profile deletion after incomplete cleanup and checks active subscriptions before withdrawing payment links or removing contacts.
Description
Out of Scope
Design
Existing Profile › Profile Edit in the Bitkit handoff. This changes failure handling; it adds no visual design.
Preview
N/A — existing error UI is reused.
QA Notes
Setup
Check out
815e85c0b2990a701999828ff2a691aff4646ff6and use its committed dependency catalog and normal devDebug build. Component tests use synthetic identities and injected service failures; no real profile, wallet or funds are needed. For J1, provision a disposable authenticated identity and a homeserver fault proxy that can hold/fail private endpoint withdrawal, lose a contact-removal acknowledgement, and return a partial acknowledgement for only some contacts, or fail profile deletion with a 404-like error without confirming the outcome. Those controls are unavailable in the standard journey fixture. For manual 3, also create and accept a future-dated recurring request with a disposable counterpart, test both payer and payee roles, and retain an unrelated saved contact. Use an open-ended disposable subscription for the cancellation case. For an accepted fixed-term subscription, preserve the existing no-cancellation policy, verify its expiry guidance, and wait for eligibility before confirming remote removal again; do not use live funds. The committedjourneys/profile/profile-cleanup-failure.xmldescribes the sequence; do not use a personal profile or reset another worker's wallet.Journeys
profile-cleanup-failure.xml— incomplete cleanup preserves retained evidence and prevents later profile deletion.Manual Tests
1. Enable the J1 cleanup fault → Edit Profile → confirm deletion → failure UI appears without profile deletion or success navigation — authenticated cleanup fault proxy not in Capabilities.
2. Inject a profile-deletion error mentioning 404 → confirm deletion → failure appears with the session and sharing settings retained — authenticated fault proxy not in Capabilities.
3. With an active recurring subscription (payer and payee cases), confirm Delete Profile or Unlink → the localized active-subscription refusal appears before endpoint/contact/profile cleanup, including for an unrelated contact. For an open-ended subscription, cancel it and confirm again → normal acknowledged removal proceeds. For fixed-term subscriptions, cancellation remains unavailable: the error explains that removal is blocked and shows the latest end time only when every blocking record has a valid finite expiry; unreadable or missing expiry remains a blocking unknown. This extends J1 with a distinct preflight case; authenticated runtime execution remains pending.
Automated Checks
PubkyRepoTest.kt— partial/unknown contact removal and unavailable reads stop before profile deletion/sign-out and preserve remaining data; direct preflight and late subscription refusals survive service-error wrapping.PaykitSdkServiceTest.kt— payer/payee and conservative expiry checks, unavailable reads, complete profile-batch refusal and later eligibility after a subscription ends.EditProfileViewModelTest.kt— private cleanup failure stops the flow; local finalization failure emits no success and does not open a deletion Retry dialog.Evidence
Controlled baseline: 221 repository tests included two expected failures for partial/unknown contact cleanup; 19 view-model tests included two expected failures for ignored cleanup outcomes. Final source passed
just compile,just test,just lintandjust format: 4,627 tests across 254 suites, zero failures/errors/skips. Complete reports contain zero Detekt/Android Lint findings. JVM sharing notices and the structured Gradle deprecation match recorded baseline diagnostics. Two consecutive independent final reviews found no outstanding issues in this scoped change.Acknowledged contact removals clear only their requested returned keys. Failed private local finalization reports an error without a deletion Retry dialog; Disconnect also consumes that result. The broader ownership/manual-retry limitation above remains open.
The follow-up at
2daec90e2removes broad profile-deletion error suppression. A controlled pre-fix repository suite executed 223 tests with one expected failure: a 404-like deletion error returned success and signed out. The repaired focused suite passed all 223 cases. Final validation passed compile, 4,628 tests across 254 suites with zero failures/errors/skips, full formatting, Detekt and Android Lint with zero findings. Complete diagnostics were inspected. Two consecutive independent final reviews are clean. The SDK already represents confirmed profile absence as normal completion; arbitrary thrown errors no longer authorize sign-out. Earlier acknowledged contact/endpoint removals are not rolled back.The previous
479c53065hosted head passed 4,627 units and all lint reports (artifact SHA-256/ZIP CRC verified, three SARIF/XML reports with zero findings). Its E2E workflow eventually passed 39 cases, but LNURL and Transfer initially timed out waiting for expected UI and passed the workflow's automatic retry. Full logs were inspected; the initial causes remain unproven. No extra rerun was requested.Gaps
J1 and manual cases 1–2 have not run as complete app journeys: the authenticated fault proxy is not provisioned. Component tests do not establish cross-device CAS or historical incident causation. The earlier three-commit range at
2daec90e2was verified locally and on GitHub (verified=true,reason=valid). The earlier second commit only renamed unchanged changelog bytes to this PR number. At historical head2daec90e2, hosted CI run 40165960501 passed 4,628 units. Lint run 40165960499 passed; complete logs and artifact 12587788708 were inspected with matching SHA-256/ZIP CRC and zero findings across three SARIF/XML reports. The manifest warning matches the actual base and previous head. That historical head's E2E run 40165960710 passed 39 cases across seven shards on their first attempts. The complete 6,672-line log was inspected. This is the existing E2E suite, not the new authenticated cleanup fault journey.At prior composed head
f72e0c03b76352a7c6db8e332b4dec7ca52b91ae, an additive signed merge incorporates parent #1688 at5fe0bf6c4f0851035632abd87dd5fb14b43507d0. All 238 cleanup feature line changes remain unchanged. The exact combined tree83ca499c244f6c5049e2a957548d7f44ed44d842passed finaljust format,just compile, unfilteredjust test(4,610 tests / 254 suites, zero failures/errors/skips) andjust lint, with zero Detekt/Android Lint findings. The sole complete diagnostic audit inspected every log and XML/SARIF/HTML report; all 1,757 frozen entries and 2,859 evidence hashes match. Three JVM notices, the manifest notice, four complete cursor-allocation stacks and the Detekt deprecation recur on the exact validated parent tree; they are retained rather than called warning-free. An independent affected-composition review is clean, with prior unchanged feature reviews retained. All four commits against the live parent are locally verified and GitHubverified=true,reason=valid. At that revision, hosted CI passed 4,610 unit tests in 254 classes with zero failures/skips, and lint has zero Detekt/Android Lint findings. E2E reached 39 distinct passes and one explicit settings skip: send_1 first failed its 30-second NavigationBack assertion, then passed automatic attempt 2. Earlier failures and process annotations are retained; their causes remain unproven. This is not an all-first-attempt pass or the authored authenticated fault journey. The complete 14 executed job logs, all unit/lint reports and four SHA-256/CRC-verified archives were inspected; the executed merge tree equals this head and its tested parent matches the live stack. Failure-only media was not uploaded after eventual success. No workflow rerun was requested. Migration was skipped, not passed. J1 and manual 1–2 remain unexecuted authenticated cases, not satisfied by unit/mock checks.Active-subscription follow-up at
7afdd3e3ba88ab5dded792073a44b4a975728ebbThe actual Edit Profile deletion path now checks raw SDK recurring obligations before private pause or cleanup. Active payer and payee records, including obligations without a saved contact, block removal; missing or unreadable end dates fail closed. The existing typed reason is normalized through service wrappers and shown with localized guidance in all 16 resource locales, without the generic Retry dialog. The repository also protects direct deletion callers. A dedicated profile contact batch rechecks subscriptions before native batch removal, preventing unrelated contacts from being removed on a known refusal. If a subscription appears after early preflight, this later refusal does not roll back already acknowledged endpoint withdrawal; no cross-device transaction claim is made.
A controlled old-path regression failed because deletion never invoked the preflight. The final combined tree passed required compile, 4,617 fresh unit tests / 254 suites with zero failures/errors/skips, full lint and final unchanged formatting. Complete diagnostics and all structured reports were inspected; Detekt and Android Lint have zero findings. All 1,756 frozen inputs matched. Earlier candidate compilation, sandbox-launch and formatter failures remain retained and are not behavioral pass evidence. Two independent source reviews found no outstanding issue in this scoped repair. Five commits against live parent
5fe0bf6c4f0851035632abd87dd5fb14b43507d0verify locally and report GitHub verified=true/reason=valid. Current-head CI passed 4,617 tests and lint has zero findings; complete logs and checksum/CRC-verified report archives were inspected. E2E finished with 39 distinct eventual passes and one explicit settings skip. Its first workflow attempt failed Docker setup on occupied port 43782 before multi-address tests began. One exact-job infrastructure retry was requested after diagnosis. That retry reached the tests, where multi_address_3 first timed out after 30 seconds waiting for “Sent” after SendSuccess/Close; its automatic second attempt passed, while cases 1/4 were marker-skipped after their earlier actual passes. Both failures remain preserved; no further rerun was requested. All 13 actual job executions and 7,617 lines were inspected without double-counting GitHub's recreated historical job IDs. The retry used the same harness and original APK digest. Twelve direct send/activity/data-source/pin files match the actual base, but shared identity dependencies changed; no base runtime equivalence or cause is claimed. Failure media were not uploaded after eventual success. Migration was skipped. These results do not satisfy authenticated cleanup J1.These checks use mocked native/network resources. J1 and manual 1–3 still need the authenticated fault/subscription fixture; no new full-app journey or historical incident-cause claim is made. Child #1693 incorporates this repair at signed
e1cc3b1717b2a4ca5365b41ff4acb954d129d1d6, preserving its original-owner/attempt orchestration. Its exact-head validation and remaining remote-Disconnect finding are documented there.Fixed-term guidance and current parent at
815e85c0b2990a701999828ff2a691aff4646ff6The typed profile-removal refusal now distinguishes aggregate subscription eligibility from ordinary contact cancellation. When all blocking records have valid finite end dates, the existing localized date is shown for the latest expiry. Missing or unreadable dates remain blocking without inventing an end time. Fixed-term cancellation policy, the all-record guard and the prohibition on cleanup after refusal are unchanged. Fresh local-only Disconnect is being integrated in child #1693; this parent does not claim that escape path is complete.
A signed additive merge includes current parent #1688 at
2aac53edc1faf2b4fd509b7e8a36701ce4289231. Exact treeed07a3f07b6765cb73246daf245ffea613e877aapassed final format, compile, 4,721 tests / 256 suites with zero failures/errors/skips, Detekt and Android Lint with zero findings. Complete 9,834-line logs and structured reports were inspected; all 1,763 frozen files and six symlinks matched. Three JVM notices, existing cursor stacks, Detekt configuration notices and cache diagnostics are retained with actual-base comparisons. Independent feature and affected-composition reviews are clean. All seven live-base commits verify locally and on GitHub. Current-head hosted CI passes 4,721 unit tests, zero failures/skips, and lint reports zero findings. E2E passes 39 cases on their first in-job attempts, with one settings suite skip. Complete 14 logs, 17 notice annotations and four checksum/CRC-verified artifacts were inspected; synthetica759400chas the exact published tree against2aac53ed. Existing JVM/cursor/manifest and generic Gradle notices remain documented; missing hosted Gradle problem details are not claimed inspected. Migration skipped. Stock E2E does not execute the new fixed-term refusal/local Disconnect journey. Prior hosted results above remain revision-scoped. Authenticated J1/manual 1–3, including the fixed-term app journey, remain unrun and are not satisfied by these tests.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)